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

Announce OtlpMeterRegistry configuration found at startup #4830

Merged

Conversation

climategadgets
Copy link
Contributor

@climategadgets climategadgets commented Mar 5, 2024

Existing Behavior
OtlpMeterRegistry says nothing at startup.

Updated Behavior
OtlpMeterRegistry emits a startup message allowing to identify and troubleshoot the OTLP receiver endpoint.

Additional Context
#4693

@pivotal-cla This is an Obvious Fix

@pivotal-cla
Copy link

@climategadgets Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@climategadgets This Pull Request contains an obvious fix. Signing the Contributor License Agreement is not necessary.

@shakuzen shakuzen changed the title Addressed #4828 (Announce OtlpMeterRegistry configuration found at startup) Announce OtlpMeterRegistry configuration found at startup Mar 11, 2024
@shakuzen shakuzen linked an issue Mar 11, 2024 that may be closed by this pull request
@shakuzen shakuzen added enhancement A general enhancement registry: otlp OpenTelemetry Protocol (OTLP) registry-related labels Mar 11, 2024
}

private void sayHello() {
logger.info("Publishing metrics for {} to {} with resource attributes {}", getClass().getSimpleName(),
Copy link
Member

Choose a reason for hiding this comment

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

Right now this will result in two log messages on start. One from PushMeterRegistry like:

publishing metrics for OtlpMeterRegistry every 1m

And now this also.

Publishing metrics for OtlpMeterRegistry to http://localhost:50901/v1/metrics with resource attributes {}

I think it may be confusing to users to see both messages. They may think two registries are publishing. I'll open a proposal to make the log message on start from PushMeterRegistry customizable and then this can be rebased on top of that if we merge it.

Copy link
Member

Choose a reason for hiding this comment

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

#4848 is now merged and these changes can be adapted to use that.

climategadgets and others added 2 commits March 12, 2024 22:43
Rather than log an additional message, customize the existing start log message.
@shakuzen shakuzen merged commit b81c63b into micrometer-metrics:main Mar 12, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A general enhancement registry: otlp OpenTelemetry Protocol (OTLP) registry-related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Announce OtlpMeterRegistry configuration found at startup
3 participants