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.

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

renaming metrics to adhere to OTEL #15087

Merged
merged 1 commit into from
Nov 4, 2024
Merged

renaming metrics to adhere to OTEL #15087

merged 1 commit into from
Nov 4, 2024

Conversation

patrickhuie19
Copy link
Contributor

@patrickhuie19 patrickhuie19 commented Nov 4, 2024 •
edited
Loading

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 4, 2024

I see you updated files related to core. Please run pnpm changeset in the root directory to add a changeset as well as in the text include at least one of the following tags:

  • #added For any new functionality added.
  • #breaking_change For any functionality that requires manual action for the node to boot.
  • #bugfix For bug fixes.
  • #changed For any change to the existing functionality.
  • #db_update For any feature that introduces updates to database schema.
  • #deprecation_notice For any upcoming deprecation functionality.
  • #internal For changesets that need to be excluded from the final changelog.
  • #nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
  • #removed For any functionality/config that is removed.
  • #updated For any functionality that is updated.
  • #wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

🎖️ No JIRA issue number found in: PR title, commit message, or branch name. Please include the issue ID in one of these.

@patrickhuie19 patrickhuie19 marked this pull request as ready for review November 4, 2024 04:16
@patrickhuie19 patrickhuie19 requested a review from a team as a code owner November 4, 2024 04:16
cedric-cordenier
cedric-cordenier reviewed Nov 4, 2024
View reviewed changes
core/services/registrysyncer/monitoring.go
@@ -16,12 +16,12 @@ var remoteRegistrySyncFailureCounter metric.Int64Counter
var launcherFailureCounter metric.Int64Counter

func initMonitoringResources() (err error) {
remoteRegistrySyncFailureCounter, err = beholder.GetMeter().Int64Counter("RemoteRegistrySyncFailure")
remoteRegistrySyncFailureCounter, err = beholder.GetMeter().Int64Counter("platform.registry_syncer.sync.failures")
Copy link
Contributor

Choose a reason for hiding this comment

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

@patrickhuie19 IIUC all metrics should be prefixed by "platform"; is that for all metrics everywhere, or is "platform" just the prefix for Keystone related things?

Copy link
Contributor Author

@patrickhuie19 patrickhuie19 Nov 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

just the prefix for Keystone related things

this 👍

@krehermann krehermann added this pull request to the merge queue Nov 4, 2024
Merged via the queue into develop with commit 2ba2f92 Nov 4, 2024
136 of 138 checks passed
@krehermann krehermann deleted the chore/keystone-metrics-rename branch November 4, 2024 14:34
@patrickhuie19 patrickhuie19 mentioned this pull request Nov 4, 2024
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cedric-cordenier cedric-cordenier cedric-cordenier approved these changes

@krehermann krehermann krehermann 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.
3 participants
@patrickhuie19 @krehermann @cedric-cordenier