Document instrumentation semantic conventions #1778
Comments
dyladan
added
the
contribfest
|
Going to take a look at |
|
👋🏽 If you're taking a look at this issue for contribfest, please comment which package you're specifically working on! |
|
will take a shot at instrumentation-amqlibs |
|
@dyladan I don't think we want the "latest" semantic conventions just yet, right, because of the breaking changes in HTTP attributes. I think we'd want to ensure we have the latest before the break (so v1.20), and then an issue/PR to add the |
|
Yes I think so |
|
Here is my understanding of the current state: Each package uses either Our package This means any packages using our I guess that is what is referenced in open-telemetry/opentelemetry-js#4235, which I'm also now realizing has a somewhat recent update to it. These two things will be very much in line. One possible approach for the immediate term would be to simply document all of these packages in this issue to say they emit semantic convention version 1.7, and if any semantic convention attribute is hard-coded in a package it should be replaced with the constant. Then separately we address open-telemetry/opentelemetry-js#4235 to update semantic conventions. Eventually we will then want to upgrade to 1.23+ with the aforementioned variables to ease pain on end users, but it seems we have some steps to take first. What do you think? |
|
I opened a PR for Express to help get this started and to have as a guide for the others. Can you take a look and confirm whether this is what we're trying to do, or if I'm missing more on this? |
I think it does exactly the right thing, thanks for taking care of that. 👍 |
|
For others that come across this and want to update for another package, this is the structure added to the Express instrumentation package, which can be used as a guide for the other packages: ## Semantic Conventions
This package uses `@opentelemetry/semantic-conventions` version `1.0+`, which implements Semantic Convention [Version 1.7.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/semantic_conventions/README.md)
Attributes collected:
| Attribute | Short Description | Notes |
| ------------ | ---------------------------------- | ----------------- |
| `http.route` | The matched route (path template). | Key: `HTTP_ROUTE` | |
JamieDanielson
changed the title
|
Updating instrumentation-document-load package #2039 to document semantic conventions |
|
@JamieDanielson (/cc @pichlermarc @maryliag @david-luna): About the Is there much value in the I think these property names on the semconv package are internal implementation details that the user of these packages doesn't need to know about. When a user uses Having that column is more just a (light) maintenance burden, and distraction. Thoughts? I realize I'm asking a little late. There are a lot of PRs for this and #2025 that are adding this table to READMEs. |
|
I agree. I believe the columns for Attribute and Descriptions are important, this is why I was creating several PRs to add them, but don't see much value on adding the key column, since it doesn't affect the usage and the user shouldn't care about it. Also, as you mention adds overhead to maintain. |
|
Yeah that's a fair point. I can see it being plenty useful to keep just the attribute key and short description, and omitting the internal key field. So instead of previous it could just be (after the exported string change, 1.0+ becomes 1.22+) ## Semantic Conventions
This package uses `@opentelemetry/semantic-conventions` version `1.0+`, which implements Semantic Convention [Version 1.7.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/semantic_conventions/README.md)
Attributes collected:
| Attribute | Short Description |
| ------------ | ---------------------------------- |
| `http.route` | The matched route (path template). | |
|
Okay, I left a comment on #2077 about this... sorry for the confusion heh but I'm okay with making the decision to omit the Notes column with the key. The attribute itself is the most important bit. @dyladan I guess let us know if you feel strongly about us keeping the Notes column, I think we've been kinda figuring it out as we go so I don't think there will be objection but wanted to check since you were the first to create this issue. Also maybe @pichlermarc |
Many packages in OTel JS Contrib export telemetry with out of date semantic conventions. Unfortunately, we have not clearly documented the version of semantic conventions implemented or which attributes are exported for instrumentations. For each package, we need to document the telemetry exported and if possible update it to the latest semantic conventions following the following template:
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.
@opentelemetry/instrumentation-amqplib@opentelemetry/instrumentation-cucumber@opentelemetry/instrumentation-dataloader@opentelemetry/instrumentation-fs@opentelemetry/instrumentation-lru-memoizer@opentelemetry/instrumentation-mongoose@opentelemetry/instrumentation-socket.io@opentelemetry/instrumentation-tedious@opentelemetry/instrumentation-aws-lambda@opentelemetry/instrumentation-aws-sdk@opentelemetry/instrumentation-bunyan@opentelemetry/instrumentation-cassandra-driver@opentelemetry/instrumentation-connect@opentelemetry/instrumentation-dns@opentelemetry/instrumentation-express@opentelemetry/instrumentation-fastify@opentelemetry/instrumentation-generic-pool@opentelemetry/instrumentation-graphql@opentelemetry/instrumentation-hapi@opentelemetry/instrumentation-ioredis@opentelemetry/instrumentation-knex@opentelemetry/instrumentation-koa@opentelemetry/instrumentation-memcached@opentelemetry/instrumentation-mongodb@opentelemetry/instrumentation-mysql@opentelemetry/instrumentation-mysql2@opentelemetry/instrumentation-nestjs-core@opentelemetry/instrumentation-net@opentelemetry/instrumentation-pg@opentelemetry/instrumentation-pino@opentelemetry/instrumentation-redis-4@opentelemetry/instrumentation-redis@opentelemetry/instrumentation-restify@opentelemetry/instrumentation-router@opentelemetry/instrumentation-winston@opentelemetry/instrumentation-document-load@opentelemetry/instrumentation-long-task@opentelemetry/instrumentation-user-interaction@opentelemetry/instrumentation-undiciThe text was updated successfully, but these errors were encountered: