-
Notifications
You must be signed in to change notification settings - Fork 42
[profilingtometrics]: remove metrics_prefix configuration option #914
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
base: main
Are you sure you want to change the base?
Conversation
| mockConsumer.AssertNotCalled(t, "ConsumeMetrics", mock.Anything, mock.Anything) | ||
| } | ||
|
|
||
| func TestConsumeProfiles_ConsumeMetricsError(t *testing.T) { |
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.
This is not needed atm, the actual ConsumeMetrics implementation cannot return an error.
| {name: "network/udp/write", | ||
| rx: regexp.MustCompile(`^(?:udp_sendmsg|udp_write|udp_push|udp_sendpage)`)}, | ||
| {name: "ipc/read", | ||
| { |
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.
Is there automated formatting in this repository that's consistent and reflects these changes? I see make gofmt but that doesn't result in additional formatting changes.
To avoid confusion with inconsistent formatting (that also adds additional review load as extra lines of code need to be checked) I suggest we first update the automated formatting in this repository to reflect these changes so that we all have a way to keep formatting consistent. Otherwise, we'll keep running into formatting inconsistencies as we use different editors with different settings.
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.
sounds good! I will create a follow-up PR for that. In any case, I removed the formatting changes for this PR f80e74d
metrics_prefix: "my.metrics"configuration option:mockMetricsConsumertest structure in favor of collector's consumertest package