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

Update usage of semantic conventions to new exported strings

Open JamieDanielson opened this issue 11 months ago • 16 comments

Same as https://github.com/open-telemetry/opentelemetry-js-contrib/issues/2025 for core repo.

This issue is intended to track the necessary change of updating each package to use the new exports for semantic conventions that were added in https://github.com/open-telemetry/opentelemetry-js/pull/4298 and now available in v1.22.0.

Each package will need to

  • update the dependency for @opentelemetry/semantic-conventions to "^1.22.0"
  • update the imported semantic convention to match the new version, i.e. HTTP_ROUTE becomes SEMATTRS_HTTP_ROUTE

Please limit PRs to a single package in order to make them quickly and easily reviewable. If you are working on one of the below packages, please comment so others know the issue is being worked on.

  • [x] @opentelemetry/shim-opentracing #4746
  • [x] @opentelemetry/sdk-trace-web #4747
  • [x] @opentelemetry/sdk-trace-node #4748
  • [x] @opentelemetry/sdk-trace-base #4749
  • [x] @opentelemetry/resources #4755
  • [x] @opentelemetry/exporter-zipkin #4756
  • [x] @opentelemetry/exporter-jaeger #4756
  • [x] @opentelemetry/core #4739
  • [x] @opentelemetry/sdk-node #4767
  • [x] @opentelemetry/instrumentation-xml-http-request #4681
  • [x] @opentelemetry/instrumentation-http #4573
  • [x] @opentelemetry/instrumentation-grpc #4633
  • [x] @opentelemetry/instrumentation-fetch #4632
  • [x] @opentelemetry/exporter-prometheus #4756
  • [ ] @opentelemetry/opentelemetry-browser-detector #4768
  • [x] examples/basic-tracer-node #4759
  • [x] examples/esm-http-ts #4758
  • [x] examples/grpc-js #4760
  • [x] examples/http #4750
  • [x] examples/https #4753
  • [ ] examples/opentelemetry-web #4764
  • [x] examples/opentracing-shim #4761
  • [x] examples/otlp-exporter-node #4762
  • [x] examples/opencensus-shim #4763

JamieDanielson avatar Mar 21 '24 14:03 JamieDanielson

I am working on http package

JamieDanielson avatar Mar 21 '24 21:03 JamieDanielson

PR for instrumentation-fetch https://github.com/open-telemetry/opentelemetry-js/pull/4632

pahiz avatar Apr 14 '24 15:04 pahiz

@JamieDanielson Can I also contribute in this issue? I can start working on this package - @opentelemetry/instrumentation-xml-http-request if no one has picked this up yet.

Prashansa-K avatar May 06 '24 06:05 Prashansa-K

Created a PR for instrumentation-xhr here: https://github.com/open-telemetry/opentelemetry-js/pull/4681

Prashansa-K avatar May 06 '24 07:05 Prashansa-K

I am working on core package.

JohannesHuster avatar May 27 '24 15:05 JohannesHuster

I'm working on

  • @opentelemetry/shim-opentracing
  • @opentelemetry/sdk-trace-web
  • @opentelemetry/sdk-trace-node
  • @opentelemetry/sdk-trace-base

JohannesHuster avatar Jun 01 '24 16:06 JohannesHuster

hey, i'm working on:

  • examples/basic-tracer-node
  • examples/esm-http-ts
  • examples/grpc-js
  • examples/http
  • examples/https
  • examples/opencensus-shim
  • examples/opentracing-shim
  • examples/otlp-exporter-node

Zen-cronic avatar Jun 02 '24 21:06 Zen-cronic

@JamieDanielson hey, i found that in each example in the examples/opentelemetry-web directory, there's no usage of @opentelemetry/semantic-conventions. But it's installed as a dep in package.json. And it's already been updated to 1.24.1. So i think the issue list can do without it!

Zen-cronic avatar Jun 02 '24 22:06 Zen-cronic

@JamieDanielson hey, i found that in each example in the examples/opentelemetry-web directory, there's no usage of @opentelemetry/semantic-conventions. But it's installed as a dep in package.json. And it's already been updated to 1.24.1. So i think the issue list can do without it!

It looks like they are being used for the service name, as seen here in esm-http-ts example. We'll need to replace SemanticResourceAttributes.SERVICE_NAME with the SEMRESATTRS_SERVICE_NAME constant instead. If it helps, take a look at this PR in the js-contrib repo for guidance.

JamieDanielson avatar Jun 03 '24 17:06 JamieDanielson

@JamieDanielson hey, i found that in each example in the examples/opentelemetry-web directory, there's no usage of @opentelemetry/semantic-conventions. But it's installed as a dep in package.json. And it's already been updated to 1.24.1. So i think the issue list can do without it!

It looks like they are being used for the service name, as seen here in esm-http-ts example. We'll need to replace SemanticResourceAttributes.SERVICE_NAME with the SEMRESATTRS_SERVICE_NAME constant instead. If it helps, take a look at this PR in the js-contrib repo for guidance.

Hey, thanks for the pointers. But i mean that none of the example in the opentelemetry-web imports @opentelemetry/semantic-conventions. For e.g., the code in the fetch example inside opentelemetry-web doesn't use the semantic-conventions package. I hope this clarifies it.

Zen-cronic avatar Jun 03 '24 18:06 Zen-cronic

Hey, thanks for the pointers. But i mean that none of the example in the opentelemetry-web imports @opentelemetry/semantic-conventions. For e.g., the code in the fetch example inside opentelemetry-web doesn't use the semantic-conventions package. I hope this clarifies it.

Oh! I see what you mean now, sorry I read too fast earlier 😅 . It looks like service names were removed inadvertently during an update and migration of this example a few years back actually, so I see we have two options: either remove the semantic conventions package from the package.json since it's not being used, or add the resource and service name for these to match the other examples. I'm leaning toward the second option, what do you think?

JamieDanielson avatar Jun 03 '24 19:06 JamieDanielson

Oh! I see what you mean now, sorry I read too fast earlier 😅 . It looks like service names were removed inadvertently during an update and migration of this example a few years back actually, so I see we have two options: either remove the semantic conventions package from the package.json since it's not being used, or add the resource and service name for these to match the other examples. I'm leaning toward the second option, what do you think?

No problem at all! Oh i see. Removing the unused dep would be straightforward, but i think we should aim for consistency with the other examples. So, i'm down with the 2nd option too.

Zen-cronic avatar Jun 03 '24 20:06 Zen-cronic

I am going to update the remaining items in the resources package

JamieDanielson avatar Jun 03 '24 21:06 JamieDanielson

Also going to update exporter-zipkin, exporter-jaeger, and exporter-prometheus

JamieDanielson avatar Jun 03 '24 21:06 JamieDanielson

i'm on the examples/opentelemetry-web now

Zen-cronic avatar Jun 04 '24 15:06 Zen-cronic

I'm working on sdk-node and opentelemetry-browser-detector.

JohannesHuster avatar Jun 05 '24 12:06 JohannesHuster