-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
postgresql receiver inconsistently does not include schemaname in tablename for some metrics #29559
Comments
Pinging code owners:
See Adding Labels via Comments if you do not have permissions to add labels yourself. |
Thanks for pointing this out @cultpony. I agree we should be consistent across the receiver. We seem to have two options here. Either have Generally I lean towards using separate attributes when we have two pieces of information. WDYT? |
Using two attributes would be definitely the better choice, IMO |
Removing |
Made a comment in the PR, but have we thought about backwards compatibility/migration path for anyone reliant on the old naming scheme? |
I mentioned it in the PR (and to bring this on the issue so that discussion isn't fragmented too much) but the best option would be to recommend users to migrate their system to rely on the newer attributes, since they are simply more reliable. To restore old behaviour a transform step in the pipeline would be sufficient: transform:
error_mode: ignore
metric_statements:
- context: resource
statements:
- set(attributes["postgresql.table.name"], Concat(attributes["postgresql.table.schema"],attributes["postgresql.table.name"],".")) |
Thanks @hughesjj for bringing up backwards compatibility, and @cultpony for putting together the PR. I saw the PR first so commented there, but really our backwards compatibility policy requires us to make breaking changes like this via a feature gate. That said, it's very helpful to have the corresponding transform for those who wish to keep the old format longer term, so thanks for putting that together as well. |
…ver (#30142) **Description:** Changes the postgresql statistics receiver so that a table or indices schema is stored in it's own attribute. Currently the schema is stored as table name for *some* of the statistics, while others completely ignore what schema a statistic comes out of. The code will allow computing things like sequential vs index scans against a table without having to pick apart the resource attribute. **Link to tracking Issue:** #29559 **Testing:** I've built the docker container for otel contrib and have been running the result for a few days to observe that the attributes are properly propagated. **Documentation:** Documentation for the new field was added to the metadata.yaml and documentation.md
…ver (open-telemetry#30142) **Description:** Changes the postgresql statistics receiver so that a table or indices schema is stored in it's own attribute. Currently the schema is stored as table name for *some* of the statistics, while others completely ignore what schema a statistic comes out of. The code will allow computing things like sequential vs index scans against a table without having to pick apart the resource attribute. **Link to tracking Issue:** open-telemetry#29559 **Testing:** I've built the docker container for otel contrib and have been running the result for a few days to observe that the attributes are properly propagated. **Documentation:** Documentation for the new field was added to the metadata.yaml and documentation.md
This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping Pinging code owners:
See Adding Labels via Comments if you do not have permissions to add labels yourself. |
Resolved by #30142 |
Component(s)
receiver/postgresql
What happened?
Description
When retreiving the metrics for index and sequential scans (postgresql.sequential_scan and postgresql.index.scan), the tablename in the metricset is inconsistently constructed;
Sequential Scan Metrics will concat the schema to the tablename (
schema.relation
), while Index.Scan will only use the Table Name, omitting the schema (relation
)You can see the issue here in these two codeblocks:
https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.88.0/receiver/postgresqlreceiver/client.go#L270
https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.88.0/receiver/postgresqlreceiver/client.go#L376
The same occurs with locks, which also do not include a schema.
The issue is that as a result, it becomes harder to actually correlate between the datasets, especially if tables exists with the same name in two schemas.
Steps to Reproduce
Expected Result
Metrics have consistent names for the resources they track.
Actual Result
Metrics have inconsistent names and cannot be reliably correlated.
Collector version
v0.88.0, v0.90.0, 3b84eae
Environment information
not relevant
OpenTelemetry Collector configuration
Log output
Additional context
I'm happy to write a patch to change the index.* metrics to use a "schema.relation" name mapping in the same way all the other metrics for postgres work. It would be nice if all metrics had consistent tagging.
The text was updated successfully, but these errors were encountered: