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

Resource sdk.version support #1311

Closed
anuraaga opened this issue Jun 5, 2020 · 6 comments · Fixed by #1366
Closed

Resource sdk.version support #1311

anuraaga opened this issue Jun 5, 2020 · 6 comments · Fixed by #1366
Assignees
Labels
Feature Request Suggest an idea for this project

Comments

@anuraaga
Copy link
Contributor

anuraaga commented Jun 5, 2020

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

Let me know if I'm missing it but I don't think the SDK fills in information about itself as described here

https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/resource/semantic_conventions/README.md#telemetry-sdk

Describe the solution you'd like

The SDK should fill in the SDK attributes. It'll probably need to propagate the current version into a Java resource file.

One thing that comes to mind is it seems to warrant exposing SDK resource attributes. Users may want to add custom attributes like here

https://github.com/aws-samples/aws-xray-sdk-with-opentelemetry-sample/blob/both-otel-and-xray/awsagentprovider/src/main/java/com/softwareaws/xray/opentelemetry/exporters/AwsTraceProvider.java#L40

And this would need some way to access the SDK resource so it can augment it, rather than replace it completely.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

@anuraaga anuraaga added the Feature Request Suggest an idea for this project label Jun 5, 2020
@anuraaga
Copy link
Contributor Author

anuraaga commented Jun 5, 2020

I'm happy to implement this if there's agreement on a general approach. Mainly found this to make sure that I was worried I might squash it.

Given the Resource is composed of many different concepts (instance, container, process, SDK) that get merged, I wonder if moving resource to its own SPI would be cleanest. Then any and all relevant ResourceProviders can contribute attributes to the final resource.

@Oberon00
Copy link
Member

Oberon00 commented Jun 5, 2020

Slightly related, one of the main arguments for #1058 is currently that it is currently very hard to explicitly set a resource that is used by the global TracerSdkProvider.

@bogdandrutu
Copy link
Member

bogdandrutu commented Jun 5, 2020

@Oberon00 Resource is an SDK component so it is not related to #1058 which allows to setup any implementation.

I would suggest probably to add a Setup method on the TraceProviderSDK that can be used to setup all the things Resource, IdGenerator, Config etc. We can have a behavior that before calling setup the SDK may be no-op or may have some defaults (we can discuss that).

@bogdandrutu
Copy link
Member

Also I think the initial issue was about the SDK automatically adding the library version to the resource, which we should do anyway.

@anuraaga
Copy link
Contributor Author

anuraaga commented Jun 6, 2020

Yeah sorry for possible noise - it was about automatically adding the library version, but in a way that doesn't clobber with user-configured resources. The latter actually seems pretty tricky :)

@carlosalberto
Copy link
Contributor

I would suggest probably to add a Setup method on the TraceProviderSDK that can be used to setup all the things Resource, IdGenerator, Config etc. We can have a behavior that before calling setup the SDK may be no-op or may have some defaults (we can discuss that).

Sounds great to me. +1 for some default values on start time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Suggest an idea for this project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants