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

[exporter/awsxray] Forward X-Ray annotations from receiver to exporter #17550

Closed
jefchien opened this issue Jan 12, 2023 · 8 comments
Closed
Assignees
Labels
enhancement New feature or request exporter/awsxray needs triage New item requiring triage receiver/awsxray

Comments

@jefchien
Copy link
Contributor

Component(s)

exporter/awsxray, receiver/awsxray

Is your feature request related to a problem? Please describe.

Currently, the X-Ray receiver receives Segments and converts them into Spans. As part of this translation, it puts the annotations into the Span's attributes along with the received Segment metadata and additional metadata. The exporter can't detect which attributes are annotations, so it depends on the configuration to contain either a list of attribute names or a flag that results in all attributes being considered annotations.

This requires a user to have knowledge of the annotations being sent by the SDK ahead of time, which isn't always possible and has an added configuration burden, or have excess and unintentional annotations indexed.

Describe the solution you'd like

X-Ray annotations that get added to the Span attributes in the X-Ray receiver could be prepended with a prefix similar to the metadata:

attrs.PutStr(
awsxray.AWSXraySegmentMetadataAttributePrefix+k, string(val))
}

This way, the exporter could detect annotations (strip prefix and add it to translated Segment) without the use of the configuration fields. The configuration fields will not be altered and will still function in the same way for backwards compatibility and usage with other non X-Ray receivers.

Describe alternatives you've considered

No response

Additional context

No response

@jefchien jefchien added enhancement New feature or request needs triage New item requiring triage labels Jan 12, 2023
@github-actions
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@jefchien jefchien changed the title Forward X-Ray annotations from receiver to exporter [exporter/awsxray] Forward X-Ray annotations from receiver to exporter Jan 12, 2023
@jefchien
Copy link
Contributor Author

Please assign this to me.

@willarmiros
Copy link
Contributor

Per offline convos, have signed off on this approach 👍

@Aneurysm9
Copy link
Member

I worry that this approach would result in unexpected behavior for users who are not exporting to X-Ray and who do not expect their attributes to be renamed. What about storing an additional attribute with a list of attribute keys that were originally annotations? Or putting that list into the context passed on to the next consumer by the receiver?

@jefchien
Copy link
Contributor Author

I figured it was acceptable to prepend attribute names since the X-Ray segment metadata already does this. If we're already renaming the metadata attributes, then users are already getting unexpected behavior when using the X-Ray receiver with a different exporter.

Or putting that list into the context passed on to the next consumer by the receiver?

Wasn't aware that you could attach values to the context. This sounds like a good solution that other exporters can just ignore.

So I would just need to wrap the context passed into the consumer with a context.WithValue?

err = x.consumer.ConsumeTraces(ctx, traces)

That seems like it would work as long as the context is maintained throughout the pipeline.

@jefchien
Copy link
Contributor Author

Discussed with @Aneurysm9 and @willarmiros. Settled on adding an attribute with a list of annotation keys.

@github-actions
Copy link
Contributor

Pinging code owners for receiver/awsxray: @willarmiros. See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions
Copy link
Contributor

Pinging code owners for exporter/awsxray: @willarmiros. See Adding Labels via Comments if you do not have permissions to add labels yourself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request exporter/awsxray needs triage New item requiring triage receiver/awsxray
Projects
None yet
Development

No branches or pull requests

4 participants