Skip to content

refactor: refactor metronome metrics with consistent tags #1241

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

Merged
merged 1 commit into from
Jun 1, 2023

Conversation

pboros
Copy link
Contributor

@pboros pboros commented May 26, 2023

Describe your changes

Refactor metronome metrics to be in-line with the rest and make sure tags won't be different in the same scope.
Right now the new metrics are additional to the old, one, needs to be deleted.

How best to test these changes

Check the metronome alerts in dev.

Issue ticket number and link

TIG-1549

@reviewpad reviewpad bot added the large label May 26, 2023
@pboros pboros force-pushed the pboros/metronome-metrics-refactor branch 8 times, most recently from 029d917 to 31aefea Compare June 1, 2023 10:36
@pboros pboros added the debug image This PR will generate a debug image label Jun 1, 2023
@pboros pboros force-pushed the pboros/metronome-metrics-refactor branch 2 times, most recently from c2f38a1 to d522585 Compare June 1, 2023 11:40
@pboros pboros added debug image This PR will generate a debug image and removed debug image This PR will generate a debug image labels Jun 1, 2023
@@ -93,6 +94,9 @@ func SchemaUpdateRepaired(project string, branch string, collection string) {
}

func InitializeMetrics() func() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How can this called twice?

@@ -93,6 +94,9 @@ func SchemaUpdateRepaired(project string, branch string, collection string) {
}

func InitializeMetrics() func() {
if metricsInitialized {
Copy link
Collaborator

@himank himank Jun 1, 2023

Choose a reason for hiding this comment

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

What about instead of this do,

var once sync.Once

InitializeMetrics() func() {
    once.Do(func() {})
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a mutex, which is basically the same as what this does internally. We need to pass io.Closer outside.

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can have io.Closer outside of Do()

@pboros pboros force-pushed the pboros/metronome-metrics-refactor branch 3 times, most recently from 2f138e5 to 4c52891 Compare June 1, 2023 16:59
himank
himank previously approved these changes Jun 1, 2023
@pboros pboros force-pushed the pboros/metronome-metrics-refactor branch from 4c52891 to 0f4294c Compare June 1, 2023 17:19
@pboros pboros force-pushed the pboros/metronome-metrics-refactor branch from 0f4294c to e6de963 Compare June 1, 2023 17:35
@pboros pboros merged commit 69f1c71 into main Jun 1, 2023
@pboros pboros deleted the pboros/metronome-metrics-refactor branch June 1, 2023 17:52
JigarJoshi added a commit that referenced this pull request Jun 1, 2023
* refactor: refactor metronome metrics with consistent tags (#1241)

* feat: Implemented API key authentication mechanism

* fix: Fixed marshaling based on code review suggestion

* feat: Added whoami endpoint

* refactor: minor refactor based on code review comments

---------

Co-authored-by: Peter Boros <[email protected]>
@tigrisdata-argocd-bot
Copy link
Collaborator

🎉 This PR is included in version 1.0.0-beta.112 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debug image This PR will generate a debug image large released on @beta
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants