Skip to content

Add system uptime metric #1507

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 25, 2024
Merged

Add system uptime metric #1507

merged 1 commit into from
Oct 25, 2024

Conversation

kevinnoel-be
Copy link
Contributor

Fixes #648

Changes

Add system.uptime metric

Merge requirement checklist

@kevinnoel-be kevinnoel-be requested review from a team as code owners October 22, 2024 11:51
@kevinnoel-be kevinnoel-be mentioned this pull request Oct 22, 2024
Closed
@mx-psi mx-psi requested a review from braydonk October 22, 2024 11:57
lmolkova
lmolkova reviewed Oct 22, 2024
View reviewed changes
@trask
Copy link
Member

trask commented Oct 22, 2024

hi @kevinnoel-be! it may be worth looking over open-telemetry/oteps#185

@kevinnoel-be kevinnoel-be requested a review from lmolkova October 23, 2024 10:00
@ChrsMark ChrsMark mentioned this pull request Oct 23, 2024
Closed
braydonk
braydonk reviewed Oct 23, 2024
View reviewed changes
Copy link
Contributor

@braydonk braydonk left a comment

Choose a reason for hiding this comment

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

LGTM, this was pretty much what I was going to do. I do have an open question to semconv maintainers, depending on their answer may ask for a change or approve.

braydonk
braydonk reviewed Oct 23, 2024
View reviewed changes
@kevinnoel-be kevinnoel-be force-pushed the add-system-uptime branch 4 times, most recently from ded7632 to 474b5ab Compare October 23, 2024 16:38
lmolkova
lmolkova approved these changes Oct 23, 2024
View reviewed changes
lmolkova
lmolkova reviewed Oct 23, 2024
View reviewed changes
@jsuereth
Copy link
Contributor

Just want to call out that we have some previous thoughts and rationale here: open-telemetry/oteps#185

braydonk
braydonk reviewed Oct 24, 2024
View reviewed changes
Copy link
Contributor

@braydonk braydonk left a comment

Choose a reason for hiding this comment

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

Based on open-telemetry/oteps#185 and the response from the semconv maintainers, I'm requesting the following changes.

jsuereth
jsuereth reviewed Oct 24, 2024
View reviewed changes
Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

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

As braydonk@ stated, I think the system.uptime should be a gauge, which DOES deviate from other time-related values for specific reasons, which is why we wrote the OTEP oh so long ago.

Otherwise looks good.

@kevinnoel-be
Copy link
Contributor Author

Adressed all the comments, PTAL again, thanks!

@trask trask mentioned this pull request Oct 25, 2024
Closed
@lmolkova lmolkova merged commit 6da7369 into open-telemetry:main Oct 25, 2024
13 of 14 checks passed
@kevinnoel-be kevinnoel-be deleted the add-system-uptime branch October 25, 2024 16:51
kevinnoel-be added a commit to collibra/opentelemetry-collector-contrib that referenced this pull request Oct 28, 2024
@kevinnoel-be
switch system.uptime to gauge
5d9b9bb 
according to semantic convention, see discussion in: open-telemetry/semantic-conventions#1507
@kevinnoel-be kevinnoel-be mentioned this pull request Oct 28, 2024
Merged
@ChrsMark ChrsMark mentioned this pull request Nov 26, 2024
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jsuereth jsuereth jsuereth approved these changes

@trask trask trask approved these changes

@lmolkova lmolkova lmolkova approved these changes

@braydonk braydonk Awaiting requested review from braydonk

Assignees

No one assigned

Labels

None yet

Projects

Semantic Conventions Triage
Archived in project
System Semantic Convention Working Group
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add system uptime metric

5 participants

@kevinnoel-be @trask @jsuereth @lmolkova @braydonk