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

[shippingservice] add resource data #504

Merged

Conversation

julianocosta89
Copy link
Member

@julianocosta89 julianocosta89 commented Oct 20, 2022

Fixes #.

Changes

Shipping Service has no resource data.
I've used the Resource Detectors in order to add some data to it.
Unfortunately telemetry.sdk data is not part of it.

Sending the PR to maybe get some insights from the everyone.

For significant contributions please make sure you have completed the following items:

  • Appropriate CHANGELOG.md updated for non-trivial changes
  • Design discussion issue #

@julianocosta89 julianocosta89 requested a review from a team as a code owner October 20, 2022 20:17
@julianocosta89
Copy link
Member Author

julianocosta89 commented Oct 20, 2022

This is what I have at the moment:
Screenshot from 2022-10-20 22-20-11

@TommyCpp and @GaryPWhite any ideas how can I add telemetry.sdk* to the spans?
Ideally we would have the same as the one mentioned in here: #497 (comment)

I've played with the ones documented here, but no luck:
https://docs.rs/opentelemetry_sdk/0.18.0/opentelemetry_sdk/resource/trait.ResourceDetector.html#tymethod.detect

@puckpuck
Copy link
Contributor

Can you update the shippingservice docs with this as well?

@julianocosta89
Copy link
Member Author

Also asked for some insights in the otel-rust slack channel.

@TommyCpp
Copy link
Contributor

open-telemetry/opentelemetry-rust#899 should do the trick

@TommyCpp
Copy link
Contributor

I need to double-check and see if we can release a minor version to include this change but feel free to copy-paste the code in the PR I linked above (or I can do it as a follow-up PR if that's preferable)

@austinlparker
Copy link
Member

Did the upstream change ever go in? Could we update deps in lieu of this PR?

@julianocosta89
Copy link
Member Author

@austinlparker updated.
@TommyCpp 's change cannot be added here because we would need a new release from the OTel Rust, which we don't have.
I don't like the approach of hardcoding the telemetry data here, so I think we could merge this one, and whenever there is a new release from the Rust side, we just update it with the TelemetryResourceDetector configuration.

@austinlparker austinlparker merged commit 34c1c6b into open-telemetry:main Nov 7, 2022
@julianocosta89 julianocosta89 deleted the shippingservice-add-resource branch November 15, 2022 06:30
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
* Add resource data to shipping service

* add changelog

* Update shippingservice docs

* Cargo update and formatting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants