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

Update usage of semantic conventions to new exported strings #4567

Closed
24 tasks done
Tracked by #4572
JamieDanielson opened this issue Mar 21, 2024 · 16 comments
Closed
24 tasks done
Tracked by #4572

Update usage of semantic conventions to new exported strings #4567

JamieDanielson opened this issue Mar 21, 2024 · 16 comments
Labels
good first issue Good for newcomers needs:code-contribution This feature/bug is ready to implement type:feature-tracking A feature with sub-issues that need to be addressed up-for-grabs Good for taking. Extra help will be provided by maintainers

Comments

@JamieDanielson
Copy link
Member

JamieDanielson commented Mar 21, 2024

Same as open-telemetry/opentelemetry-js-contrib#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 #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.

@JamieDanielson JamieDanielson added good first issue Good for newcomers type:feature-tracking A feature with sub-issues that need to be addressed up-for-grabs Good for taking. Extra help will be provided by maintainers needs:code-contribution This feature/bug is ready to implement labels Mar 21, 2024
@JamieDanielson
Copy link
Member Author

I am working on http package

@pahiz
Copy link
Contributor

pahiz commented Apr 14, 2024

PR for instrumentation-fetch #4632

@Prashansa-K
Copy link
Contributor

@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
Copy link
Contributor

Created a PR for instrumentation-xhr here: #4681

@JohannesHuster
Copy link
Contributor

I am working on core package.

@JohannesHuster
Copy link
Contributor

I'm working on

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

@Zen-cronic
Copy link
Contributor

Zen-cronic commented Jun 2, 2024

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
Copy link
Contributor

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

@JamieDanielson
Copy link
Member Author

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

@Zen-cronic
Copy link
Contributor

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

@JamieDanielson
Copy link
Member Author

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?

@Zen-cronic
Copy link
Contributor

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.

@JamieDanielson
Copy link
Member Author

JamieDanielson commented Jun 3, 2024

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

@JamieDanielson
Copy link
Member Author

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

@Zen-cronic
Copy link
Contributor

i'm on the examples/opentelemetry-web now

@JohannesHuster
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers needs:code-contribution This feature/bug is ready to implement type:feature-tracking A feature with sub-issues that need to be addressed up-for-grabs Good for taking. Extra help will be provided by maintainers
Projects
None yet
Development

No branches or pull requests

5 participants