Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add useragent to awsemfexporter #35748

Closed

Conversation

harrryr
Copy link
Contributor

@harrryr harrryr commented Oct 11, 2024

Description

Cherry-picking from downstream: amazon-contributing#170

Changes include:

  • Adds additional user agent for awsemfexporter
  • Extracts the SDK Telemetry attributes from the resource attributes to build the user agent strings.
  • Uses a TTL cache to keep track of the language/version map received by the exporter.

Some additional changes were included:

  • Modified so that SDK Telemetry attributes are generated for all SDKs, not just for App Signals
  • Modified code based on comments from previous cherry-picking attempt: (PR)

Testing:

Added unit test

@harrryr harrryr requested a review from a team as a code owner October 11, 2024 19:08
@harrryr harrryr requested a review from mwear October 11, 2024 19:08
@harrryr harrryr force-pushed the add-awsemflogexporter-user-agent branch 2 times, most recently from 681e09f to 03723d0 Compare October 11, 2024 20:31
@harrryr harrryr force-pushed the add-awsemflogexporter-user-agent branch from 03723d0 to fc991c8 Compare October 11, 2024 20:48
@harrryr harrryr force-pushed the add-awsemflogexporter-user-agent branch from 6acab1b to d3c54f0 Compare October 14, 2024 21:46
Copy link
Member

@Aneurysm9 Aneurysm9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I continue to have the same concerns I previously expressed regarding use of the user agent header to exfiltrate information about the workload being processed.

language = truncate(language, attrLengthLimit)
version = truncate(version, attrLengthLimit)
value := ua.cache.Get(language)
valueMap := make(map[string]struct{})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name valueMap may be a bit confusing, given that we are using this value to store a set of versions.
Can we update the variable name (e.g. versionSet) and add comment that we are treating the map as a set?

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Oct 29, 2024
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants