opentelemetry-demo icon indicating copy to clipboard operation
opentelemetry-demo copied to clipboard

[shippingservice] add resource data

Open julianocosta89 opened this issue 1 year ago • 5 comments

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:

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

julianocosta89 avatar Oct 20 '22 20:10 julianocosta89

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: https://github.com/open-telemetry/opentelemetry-demo/pull/497#issue-1415732281

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

julianocosta89 avatar Oct 20 '22 20:10 julianocosta89

Can you update the shippingservice docs with this as well?

puckpuck avatar Oct 21 '22 01:10 puckpuck

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

julianocosta89 avatar Oct 21 '22 12:10 julianocosta89

https://github.com/open-telemetry/opentelemetry-rust/pull/899 should do the trick

TommyCpp avatar Oct 23 '22 04:10 TommyCpp

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)

TommyCpp avatar Oct 25 '22 05:10 TommyCpp

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

austinlparker avatar Nov 03 '22 17:11 austinlparker

@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.

julianocosta89 avatar Nov 05 '22 14:11 julianocosta89