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

X-Ray ID Generator should be split from X-Ray Propagator Package #33

Closed
NathanielRN opened this issue Jan 25, 2022 · 2 comments
Closed
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@NathanielRN
Copy link
Contributor

NathanielRN commented Jan 25, 2022

Description

Today, the X-Ray ID Generator lives inside the opentelemetry-propagator-xray gem instead of being in its own package.

This is a problem for projects that want to install either the Propagator or the ID Generator, but not the other. For example, the https://github.com/open-telemetry/opentelemetry-lambda repository creates Lambda Layers that can be used to export to any backend. The specifications for instrumenting an AWS Lambda function say it should use the AWS X-Ray Propagator and so Lambda Instrumentation packages normally take it as a dependency.

However, Lambda Instrumentations should not be taking the AWS X-Ray ID Generator a dependency, since it is not required. Because of the way the OTel Ruby opentelemetry-propagator-xray gem is set up, it will have no choice but to take the ID Generator.

Instead, we should split the ID Generator into its own gem called opentelemetry-sdk-extension-xray, because it is meant to "extend" on the OTel SDK's notion of "Id Generators". (I know in OTel Ruby the concept of an "ID Generator" is directly in the opentelemetry-api package, but the specification describes Id Generator as only an SDK concept, not an API concept).

This would match what other OpenTelemetry SIGs are doing:

OTel JS:

OTel Java:

OTel Python

@NathanielRN NathanielRN added the bug Something isn't working label Jan 25, 2022
@arielvalentin arielvalentin added the help wanted Extra attention is needed label Jan 25, 2022
@fbogsany
Copy link
Contributor

I know in OTel Ruby the concept of an "ID Generator" is directly in the opentelemetry-api package, but the specification describes Id Generator as only an SDK concept, not an API concept

To be clear, the default ID generator is part of the OpenTelemetry::Trace module in OTel Ruby because it is required by SpanContext, but the pluggable ID generator concept is part of the SDK, in the SDK's TracerProvider.

Given the discrepancy between Python and Java on one side and JS on the other, it seems like the weight of SIGs is on the side of a SDK extension package for AWS that contains both the ID generator and AWS resource detectors.

@plantfansam plantfansam transferred this issue from open-telemetry/opentelemetry-ruby Jun 14, 2022
dlahn added a commit to dlahn/opentelemetry-ruby-contrib that referenced this issue Mar 17, 2023
This implements open-telemetry#33. This adds a gem opentelemetry-sdk-extension-aws, which allows for XRay ID generation. This lays the groundwork to add resource detectors for AWS resources.

This code is mostly moved from the opentelemetry-propagator-xray gem.
dlahn added a commit to dlahn/opentelemetry-ruby-contrib that referenced this issue Mar 17, 2023
This implements open-telemetry#33. This adds a gem opentelemetry-sdk-extension-aws, which allows for XRay ID generation. This lays the groundwork to add resource detectors for AWS resources.

This code is mostly moved from the opentelemetry-propagator-xray gem.
dlahn added a commit to dlahn/opentelemetry-ruby-contrib that referenced this issue Mar 17, 2023
This implements open-telemetry#33. This adds a gem opentelemetry-sdk-extension-aws, which allows for XRay ID generation. This lays the groundwork to add resource detectors for AWS resources.

This code is mostly moved from the opentelemetry-propagator-xray gem.

feat: Add SDK Extension for AWS

This implements open-telemetry#33. This adds a gem opentelemetry-sdk-extension-aws, which allows for XRay ID generation. This lays the groundwork to add resource detectors for AWS resources.

This code is mostly moved from the opentelemetry-propagator-xray gem.
dlahn added a commit to dlahn/opentelemetry-ruby-contrib that referenced this issue Mar 17, 2023
This implements open-telemetry#33. This adds a gem opentelemetry-sdk-extension-aws, which allows for XRay ID generation. This lays the groundwork to add resource detectors for AWS resources.

This code is mostly moved from the opentelemetry-propagator-xray gem.
@ericmustin
Copy link
Contributor

There has been some back and forth discussion over this and practically speaking the otel ruby sig has decided that an aws-extension gem is not something we plan to prioritize given our current time commitments. The nice-to-haves it offers seem reasonable but none of them appear to be must-haves, either according to the specification or for any practical use cases that have been identified so far. The propagator gem can be used to pull in the id generator, and the aws resource detector gem is something users plan to contribute as either a standalone gem or part of our broad "resource_detectors" gem.

We are closing this issue to prevent any confusion going forward. If this becomes required by the specification or someone from AWS wants to contribute and maintain this gem, we're happy to reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants