Remove dependency on @opentelemetry/semantic-conventions
#5397
Conversation
🦋 Changeset detectedLatest commit: 0132356 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
IMax153
changed the title
@opentelemetry/semantic-conventions
| const ATTR_HTTP_REQUEST_HEADER = (key: string): string => `http.request.header.${key}` | ||
| const ATTR_HTTP_REQUEST_METHOD = "http.request.method" | ||
| const ATTR_HTTP_RESPONSE_HEADER = (key: string): string => `http.response.header.${key}` | ||
| const ATTR_HTTP_RESPONSE_STATUS_CODE = "http.response.status_code" | ||
| const ATTR_SERVER_ADDRESS = "server.address" | ||
| const ATTR_SERVER_PORT = "server.port" | ||
| const ATTR_URL_FULL = "url.full" | ||
| const ATTR_URL_PATH = "url.path" | ||
| const ATTR_URL_SCHEME = "url.scheme" | ||
| const ATTR_URL_QUERY = "url.query" |
There was a problem hiding this comment.
Question: I'm wondering if it would be appropriate to include an inline comments above each of those const declaration groups that links to the relevant documentation online where someone could find out what all semantic keys are derived from.
There was a problem hiding this comment.
We are debating how to best handle semantic OTEL attributes. For now, this will unblock the bugs we are facing with ESM builds but are looking at a larger strategy. Once we have one in place we will document accordingly. I avoided doing it here to prevent drift.
😭 #5192 |
|
If you change your mind and decide to add comments, let me do it. I want to do it right at least this time. I can make a PR adding comments with links. In the end, as a thing to consider, maybe it will be a good idea to create an |
We were actually just discussing something like this internally, but instead of creating a new package, keeping everything in our existing opentelemetry package given the OTEL deps are peer dependencies there. May make sense as a separate package but we really haven't come to a conclusion yet. The OTEL folks look like they autogenerate a lot of the semantic conventions code from the spec so it's probably not too difficult to maintain. @nikelborm if you are interested maybe you could spec something out in the existing OTEL package and we can separate it out if we decide it make sense? |
|
Could you please elaborate on what kinds of ESM problems
By that, do you mean in the upstream repo (open-telemetry/opentelemetry-js) of |
Our users have consistently run into a variety of issues when building due to OTEL dependencies coming from our packages due to the way the libraries are published. It's highly specific to build environment. We would rather get rid of this friction point moving forward. I'll let @mikearnaldi provide more context on the concrete issues as he has dealt with more of this than I have. |
all |
|
I found a few PRs that promise to fix the situation with ESM.
And they seem to be quite active. Will you be willing to reconsider removing dependency on |
|
Probably not given our experience in the past and the lack of guarantee that this will fix all underlying issues. |
So, by that, I assume the answer to my previous question:
is no? I'm a bit confused about what you meant by |
|
Yea I was referring to our |
|
Our objective is to have the Effect DX being smooth, for the time being official otel packages are a liability rather than an asset, if that changes over time and I truly hope it will we will reconsider. Note that this doesn’t mean dropping support for official otel packages, rather it means providing alternatives |
Type
Description
The OpenTelemetry packages have issues with ESM builds. This PR removes the
@opentelemetry/semantic-conventionsdependency from all packages except the@effect/opentelemetryintegration package in favor of statically declared attribute names / values.Related