-
Notifications
You must be signed in to change notification settings - Fork 5
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
chore: Remove the Metrics name dedup #22
Conversation
Signed-off-by: Saravanan Balasubramanian <[email protected]>
Signed-off-by: Saravanan Balasubramanian <[email protected]>
I created the issue to add test case for the Prometheus push. We need to find the test library to test Prometheus abstraction. we need to refactor the code |
for _, payload := range msgPayloads { | ||
if _, ok := keys[payload.Name]; !ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name level dedub was removed
prometheus-pusher/main.go
Outdated
return err | ||
} | ||
p.metrics.IncreaseTotalSuccess() | ||
p.logger.Debug("Pushing Payload ", zap.Any("payload", payload)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debugw
}) | ||
appName := payload.Labels["app"] | ||
p.metrics.IncreaseAnomalyGenerated(payload.Namespace, appName, payload.Name) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logically, missing default
?
Signed-off-by: Saravanan Balasubramanian <[email protected]>
} | ||
p.metrics.IncreaseTotalSuccess() | ||
p.logger.Debug("Pushing Payload ", zap.Any("payload", payload)) | ||
pusher, err := p.createPusher(fmt.Sprintf("%s_%s_%s", payload.Namespace, payload.Subsystem, payload.Name)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to double check: do we need to create a new pusher for each payload item?
p.logger.Errorw("Unsupported Metrics Type", zap.Any("payload", payload)) | ||
return fmt.Errorf("unsupported Metrics Type") | ||
} | ||
err = pusher.Push() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to double check: if a pusher needs an explicit close or such call.
} | ||
p.logger.Infow("Successfully pushed", zap.Any("payload", payload)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would this infow() generate too many log lines in regular cases?
No description provided.