Change network.peer.address to opt-in on http.client.open_connections (related to #3279)#3759
Change network.peer.address to opt-in on http.client.open_connections (related to #3279)#3759cijothomas wants to merge 2 commits into
Conversation
|
This PR contains changes to area(s) that do not have an active SIG/project and will be auto-closed:
Such changes may be rejected or put on hold until a new SIG/project is established. Please refer to the Semantic Convention Areas |
|
@open-telemetry/specs-semconv-maintainers This PR was auto-closed by a bot because the HTTP SIG is marked inactive in AREAS.md. But that doc explicitly says "Bugs and bugfixes are welcome", so this PR should not have been auto-closed. (Its fixing an issue, not sure if it is a bug, in a in-development metric) Also, this bot action doesn't seem very welcoming to contributors:
A contributor (especially a newcomer) following the docs would have no way to anticipate this. Would it make sense to either make this more obvious in contributing/areas.md docs to avoid the shock? In the meantime, could a maintainer reopen this PR? Happy to address any review feedback for this one. |
|
@lmolkova I kept the attribute as recommended in the connection.duration metric as it is an histogram, and the original issue only occurs when UpDownCounter is involved (due to its always-cumulative nature). Please confirm if this is acceptable. (I saw you approved, but want to call this one out specifically) |
|
@cijothomas so if I understand correctly:
If it's correct than I think it makes sense to make it opt-in on the updowncounter and keep on histogram. I added a comment to #1854 tracking general metric guidance to potentially document it. |
Your explanation is correct. Histogram also would have this problem of never forgetting the 1.2.3.4 entry, if user opts for Cumulative. With delta, Histogram "forgets". UpDownCounter is somewhat special - even when user says DELTA, UpDownCounter actually still does cumulative. i.e - a normal user has no easy way to make UpDownCounter behave in DELTA mode! (it is possible for an advanced user to control this, but that might defeat the entire point of UpDownCounter. I guess these are spec problems, and should be addressed there.) |
Towards #3279 (does not fully resolve it).
http.client.open_connectionsis a synchronousUpDownCounter, which is commonly exported with cumulative temporality. Combined withnetwork.peer.addressatrecommended, this produces one persistent time series per remote IP for the lifetime of the process — even after connections close — leading to unbounded SDK memory growth, exhaustion of cardinality limits, and collapse of useful low-cardinality attributes (e.g.url.scheme) into the overflow bucket.This PR moves
network.peer.addressfromRecommendedtoOpt-Inonhttp.client.open_connections.A deeper fix will require a spec change. The proposed
finish()/ remove-series API (open-telemetry/opentelemetry-specification#4702) may also help address this in the future by letting instrumentations explicitly drop a series when the underlying entity (e.g. a connection) goes away./cc @open-telemetry/semconv-http-approvers @lmolkova @ManickaP @noahfalk