Skip to content
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

refactor: separate metrics code for reuse #13514

Merged
merged 1 commit into from
Sep 5, 2024
Merged

refactor: separate metrics code for reuse #13514

merged 1 commit into from
Sep 5, 2024

Conversation

Joibel
Copy link
Member

@Joibel Joibel commented Aug 27, 2024

This PR is intended as a pure refactor to move code.

It separates the reusable metrics code into something that could be reused in a separate module.

This intention is proved in a separate PR which adds metrics to the server. #13515 should be used as a reference for how this would work.

The version metric is moved to also be reusable as it is simple and obvious to be reused in the server.

Labels are renamed to Attributes as per OpenTelemetry.

@Joibel Joibel mentioned this pull request Aug 27, 2024
@Joibel Joibel added area/controller Controller issues, panics area/metrics labels Aug 27, 2024
@Joibel Joibel force-pushed the telemetry-refactor branch 6 times, most recently from 4c980f2 to c4c52e2 Compare August 30, 2024 15:14
This PR is intended as a pure refactor to move code.

It separates the reusable metrics code into something that could be
reused in a separate module.

This intention is proved in a separate PR which adds metrics to the
server.

The `version` metric is moved to also be reusable as it is simple and
obvious to be reused in the server.

Labels are renamed to Attrib(utes) to match OpenTelemetry.

Signed-off-by: Alan Clucas <[email protected]>
@Joibel Joibel force-pushed the telemetry-refactor branch from c4c52e2 to 33d9159 Compare August 30, 2024 15:52
@Joibel Joibel marked this pull request as ready for review August 30, 2024 15:52
Copy link
Member

@isubasinghe isubasinghe left a comment

Choose a reason for hiding this comment

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

I think the environment-variables.md file needs to be updated as well.

util/telemetry/instrument.go Show resolved Hide resolved
util/telemetry/metrics.go Show resolved Hide resolved
@Joibel Joibel requested a review from isubasinghe September 4, 2024 13:28
Copy link
Member

@isubasinghe isubasinghe left a comment

Choose a reason for hiding this comment

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

LGTM

@Joibel Joibel merged commit 7173a27 into main Sep 5, 2024
28 checks passed
@Joibel Joibel deleted the telemetry-refactor branch September 5, 2024 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/controller Controller issues, panics area/metrics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants