[receiver/apache] Add opt-in migration to new metric and attribute names behind feature gates - #47367
[receiver/apache] Add opt-in migration to new metric and attribute names behind feature gates#47367singhvibhanshu wants to merge 26 commits into
Conversation
Signed-off-by: singhvibhanshu <singhvibhanshu@hotmail.com>
Signed-off-by: singhvibhanshu <singhvibhanshu@hotmail.com>
|
Hi there! |
Signed-off-by: singhvibhanshu <singhvibhanshu@hotmail.com>
|
This PR got merge conflicts. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Signed-off-by: singhvibhanshu <singhvibhanshu@hotmail.com>
Signed-off-by: singhvibhanshu <singhvibhanshu@hotmail.com>
Signed-off-by: singhvibhanshu <singhvibhanshu@hotmail.com>
edmocosta
left a comment
There was a problem hiding this comment.
Let's please close #47367 (review) before merging.
|
Understood, @edmocosta. But before proceeding let me confirm everything one more time. Do you mean introducing feature gates here? I have a few questions to ask:
|
Normally yes. There are other strategies components can take like emitting both metrics old and new during the transition time, but that's up to the code-owners @colelaven & @ishleenk17 to decide, we just need to ensure it's not breaking existing users independently of the strategy.
Yes, the old metrics and attributes names should be the default ones while the FF is still in alpha. The new names should be opt-in on this phase.
The warning message should be logged when the component starts and the FG is disable (default). It should link to the migration's docs, explaining how users can make this transition, what they should look at (OTTL statements, dashboards, alerts, routing, etc), and what exactly changed.
Yes. Please also take into consideration the FG lifecycle described on the guidelines, and the versions timline recommendations:
|
|
FWIWI, here is the RFC that describes the migration mechanism for when there are breaking changes and we aim to move to stable semconv: https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/rfcs/semconv-feature-gates.md I would suggest that such discussions, specially when breaking changes are involved, should happen first at the respective issue (#47327) and involve approvers/maintainers for sponsoring these breaking changes if there is a specific plan. @singhvibhanshu few questions from side that should be discussed/answered back at the issue. Is there a specific goal for this change? Have code-owners agreed on this change and what is the specific goal? This kind of updates/changes are not trivial since they introduce breaking changes on components that are already in use and even if the stability level of the component is not at |
Understood.
The goal of this issue is to align the metric names with the established semantic conventions. Regarding the code owner approval, Cole has already approved this PR. If anything seems off to him, I think he would let me know.
Duly noted. I'll keep in mind that this won't happen again. Also, I'm updating the description of this PR. |
Something that needs to be clarified is the level of confidence for these "established" semantic conventions and what are the plans of stabilising those. Ideally we want to have breaking changes only once for components, so we should ensure that semantic conventions we move to are actually there to stay and won't change again. I would suggest we move the discussion back to the issue to work on the plan first before doing any implementation. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Signed-off-by: singhvibhanshu <find.vibhanshu@gmail.com>
Signed-off-by: singhvibhanshu <find.vibhanshu@gmail.com>
| monotonic: false | ||
| aggregation_temporality: cumulative | ||
| attributes: [] | ||
| apache.connections: # (new) |
There was a problem hiding this comment.
| apache.connections: # (new) | |
| apache.connection.status: # (new) |
As per semconv guidance.
There was a problem hiding this comment.
I donot agree with this. Please see: #47367 (comment)
There was a problem hiding this comment.
Take a look at https://opentelemetry.io/docs/specs/semconv/how-to-write-conventions/status-metrics/, the key thing is the presence of the state attribute.
It also becomes awkward that we have a connection namespace however the count of connections based on state is not in that namespace.
There was a problem hiding this comment.
Agreed, thanks for the reference. Let's not proceed further and wait for the code-owner to have a look too.
@colelaven, please take a look.
There was a problem hiding this comment.
Thanks for the reference Thompson, I am in favor of renaming back to apache.connection.status after reading that document 👍
Pull request dashboard statusStatus last refreshed: 2026-07-27 13:30:13 UTC.
This automated status or its linked feedback items may be incorrect. If something looks wrong, please report it with the result you expected. |
Signed-off-by: singhvibhanshu <find.vibhanshu@gmail.com>
|
/rerun |
Signed-off-by: singhvibhanshu <find.vibhanshu@gmail.com>
|
@thompson-tomo, @colelaven |
Signed-off-by: singhvibhanshu <find.vibhanshu@gmail.com>
Description
This PR offers an opt-in, non-breaking migration to new, more consistent metric and attribute names for the Apache receiver.
NOTE: It's a naming consistency cleanup, not conformance to a published Apache semantic conventions (as it doesn't' exist as of today).
Metrics are renamed as follows (and are emitted only when
enableNewFormatMetricsis on):apache.current_connections->apache.connection.activeapache.connections.async->apache.connection.statusapache.requests->apache.request.countapache.scoreboard->apache.worker.statusapache.workers-> split intoapache.worker.active(busy) +apache.worker.idle(idle)Attributes are renamed as:
connection_state->apache.connection.statescoreboard_state->apache.worker.statelevel->apache.process.levelmode->cpu.modeworkers_state-> removed (bec old apache.workers split into attribute-less metrics)Link to tracking issue
Fixes #47327
Testing
Added.
Documentation
Added.
Authorship