Skip to content

Export internal metrics to follow dot notation#1147

Merged
christos68k merged 19 commits intoopen-telemetry:mainfrom
coralogix:nimrodavni78/rename-internal-metrics
Mar 14, 2026
Merged

Export internal metrics to follow dot notation#1147
christos68k merged 19 commits intoopen-telemetry:mainfrom
coralogix:nimrodavni78/rename-internal-metrics

Conversation

@NimrodAvni78
Copy link
Copy Markdown
Contributor

@NimrodAvni78 NimrodAvni78 commented Feb 2, 2026

resolves #1115

@NimrodAvni78 NimrodAvni78 requested review from a team as code owners February 2, 2026 15:07
@NimrodAvni78 NimrodAvni78 changed the title trename internal metrics to follow semantic conventions Rename internal metrics to follow semantic conventions Feb 2, 2026
christos68k
christos68k reviewed Feb 2, 2026
View reviewed changes
Comment thread metrics/genids/main.go Outdated
florianl
florianl reviewed Feb 3, 2026
View reviewed changes
Comment thread metrics/metrics.json Outdated
},
{
"description": "Number of failures to read from pyinfo->pyThreadStateCurrentAddr",
"description": "Number of failures to read from pyinfo-\u003epyThreadStateCurrentAddr",
Copy link
Copy Markdown
Member

@florianl florianl Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we revert these changes to \u003e?

@NimrodAvni78 NimrodAvni78 force-pushed the nimrodavni78/rename-internal-metrics branch 2 times, most recently from 32ac664 to c8522a5 Compare February 3, 2026 09:18
@NimrodAvni78 NimrodAvni78 force-pushed the nimrodavni78/rename-internal-metrics branch from c8522a5 to db9ed11 Compare February 3, 2026 09:20
florianl
florianl approved these changes Feb 4, 2026
View reviewed changes
Comment thread metrics/metrics.json Outdated
christos68k
christos68k approved these changes Feb 4, 2026
View reviewed changes
Comment thread metrics/metrics.json Outdated
"type": "counter",
"name": "Invalid",
"field": "",
"field": "invalid",
Copy link
Copy Markdown
Member

@christos68k christos68k Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to set this, it'll never be sent and it's confusing if we give it a field value

Suggested change
"field": "invalid",
"field": "",

Copy link
Copy Markdown
Member

@christos68k christos68k Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NimrodAvni78 Let's address this too

Copy link
Copy Markdown
Contributor Author

@NimrodAvni78 NimrodAvni78 Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christos68k after adding validation that field can't be empty this is not valid and initilization panics
do you think we should keep it as "invalid"?

Copy link
Copy Markdown
Member

@christos68k christos68k Mar 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a suggestion here.

We also need to remove the panic (see: #888) but I can do that in a follow-up PR.

rogercoll
rogercoll requested changes Feb 5, 2026
View reviewed changes
Comment thread metrics/metrics.go
@florianl florianl requested a review from rogercoll February 10, 2026 07:22
@rogercoll rogercoll mentioned this pull request Feb 11, 2026
Open
rogercoll
rogercoll reviewed Feb 11, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@rogercoll rogercoll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The metric field names in metrics/metrics.json are mostly well-structured, but have several areas of non-compliance with the OpenTelemetry Semantic Convention naming guidelines (​https://opentelemetry.io/docs/specs/semconv/general/naming/​) and metrics conventions (​https://opentelemetry.io/docs/specs/semconv/general/metrics/​).

I agree on merging this PR, but I would suggest not using semantic conventions as commit message. Instead: Export internal metrics to follow dot notation.

This PR corresponds to the first task to migrate to OpenTelemetry naming, see #1115 (comment)

Comment thread metrics/metrics.json Outdated
Comment thread metrics/metrics.json Outdated
Comment thread metrics/metrics.json Outdated
Comment thread metrics/metrics.json Outdated
Comment thread metrics/metrics.json Outdated
@christos68k christos68k self-requested a review February 11, 2026 11:19
@christos68k
Copy link
Copy Markdown
Member

christos68k commented Feb 17, 2026

@NimrodAvni78 Can you address the latest round of comments so that we can move forward with this PR? Thanks!

@NimrodAvni78 NimrodAvni78 changed the title Rename internal metrics to follow semantic conventions Export internal metrics to follow dot notation Feb 17, 2026
NimrodAvni78 and others added 6 commits February 17, 2026 13:14
@NimrodAvni78 @rogercoll
Update metrics/metrics.json
6822eb5 
Co-authored-by: Roger Coll <roger.coll@elastic.co>
@NimrodAvni78 @rogercoll
perf.event.no.data.munmap -> perf_event.no.data.munmap
24582ca 
Co-authored-by: Roger Coll <roger.coll@elastic.co>
@NimrodAvni78 @rogercoll
perf.event.read.error.munmap -> perf_event.read.error.munmap
d23ca6e 
Co-authored-by: Roger Coll <roger.coll@elastic.co>
@NimrodAvni78 @rogercoll
perf.event.enable.err -> perf_event.enable.err
ec1693b 
Co-authored-by: Roger Coll <roger.coll@elastic.co>
@NimrodAvni78 @rogercoll
perf.event.disable.err -> perf_event.disable.err
27cd5b9 
Co-authored-by: Roger Coll <roger.coll@elastic.co>
@NimrodAvni78
Merge branch 'main' into nimrodavni78/rename-internal-metrics
edb0939
@NimrodAvni78
Copy link
Copy Markdown
Contributor Author

NimrodAvni78 commented Feb 17, 2026 •
edited
Loading

I agree on merging this PR, but I would suggest not using semantic conventions as commit message. Instead: Export internal metrics to follow dot notation.

This PR corresponds to the first task to migrate to OpenTelemetry naming, see #1115 (comment)

done 👌. @rogercoll do you want to to also change the issue name? or change the description of this PR to not close the issue?

@NimrodAvni78
Copy link
Copy Markdown
Contributor Author

NimrodAvni78 commented Feb 17, 2026

@christos68k just merged all suggestions. so should be good to go

@rogercoll
Copy link
Copy Markdown
Contributor

rogercoll commented Feb 18, 2026

I agree on merging this PR, but I would suggest not using semantic conventions as commit message. Instead: Export internal metrics to follow dot notation.
This PR corresponds to the first task to migrate to OpenTelemetry naming, see #1115 (comment)

done 👌. @rogercoll do you want to to also change the issue name? or change the description of this PR to not close the issue?

Thanks @NimrodAvni78 , no worries, we can keep the issue as is and re-open it afterwards.

@NimrodAvni78
Copy link
Copy Markdown
Contributor Author

NimrodAvni78 commented Mar 8, 2026

@christos68k @rogercoll do you think this could get merged?
is there any follow up work needed on this?

@christos68k
Copy link
Copy Markdown
Member

christos68k commented Mar 9, 2026 •
edited
Loading

@christos68k @rogercoll do you think this could get merged? is there any follow up work needed on this?

Hi @NimrodAvni78, there are a few comments you haven't addressed, once you do we can merge this PR.

NimrodAvni78 and others added 3 commits March 11, 2026 19:20
@NimrodAvni78 @florianl
Update metrics/metrics.json
00f1758 
Co-authored-by: Florian Lehner <florianl@users.noreply.github.com>
@NimrodAvni78 @christos68k
Update metrics/metrics.json
2eb8ebb 
Co-authored-by: Christos Kalkanis <christos.kalkanis@elastic.co>
@NimrodAvni78
Merge branch 'main' into nimrodavni78/rename-internal-metrics
0594a61
christos68k
christos68k reviewed Mar 12, 2026
View reviewed changes
Comment thread metrics/metrics.go Outdated
@florianl florianl self-requested a review March 13, 2026 11:50
@NimrodAvni78 @christos68k
Update metrics/metrics.go
4957e43 
Co-authored-by: Christos Kalkanis <christos.kalkanis@elastic.co>
@christos68k christos68k merged commit b8ee909 into open-telemetry:main Mar 14, 2026
32 checks passed
@gnurizen gnurizen mentioned this pull request Mar 23, 2026
Closed
This was referenced Apr 14, 2026
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@christos68k christos68k christos68k approved these changes

@florianl florianl florianl approved these changes

@rogercoll rogercoll rogercoll approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Change internal metric names to follow naming conventions

4 participants

@NimrodAvni78 @christos68k @rogercoll @florianl