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

Add WARN message when OtlpMeterRegistry fails to publish metrics #4271

Merged
merged 1 commit into from
Oct 25, 2023
Merged

Add WARN message when OtlpMeterRegistry fails to publish metrics #4271

merged 1 commit into from
Oct 25, 2023

Conversation

antechrestos
Copy link
Contributor

@antechrestos antechrestos commented Oct 24, 2023

Experienced this bad behaviour while plugging my application to an APM; as I configured the bad endpoint, it answered me with 403 http status, yet nothing appeared in logs, as if everything was normal. I had to use debugger to see the status.

@pivotal-cla
Copy link

@antechrestos Please sign the Contributor License Agreement!

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

See the FAQ for frequently asked questions.

@antechrestos antechrestos changed the title fixes(logging) add warning message when 'OtlpMeterRegistry' fails to … fixes(logging) add warning message when 'OtlpMeterRegistry' fails to export metrics Oct 24, 2023
@pivotal-cla
Copy link

@antechrestos Thank you for signing the Contributor License Agreement!

@jonatan-ivanov
Copy link
Member

Thank you for the PR!
I think this makes sense. I believe we should do this on the 1.10.x branch so not only 1.12 but 1.11 and 1.10 will get this fix. Could you please rebase your changes on the 1.10.x branch?

@jonatan-ivanov jonatan-ivanov added this to the 1.10.x milestone Oct 24, 2023
@jonatan-ivanov jonatan-ivanov changed the title fixes(logging) add warning message when 'OtlpMeterRegistry' fails to export metrics Add WARN message when OtlpMeterRegistry fails to publish metrics Oct 24, 2023
@jonatan-ivanov jonatan-ivanov added the bug A general bug label Oct 24, 2023
@jonatan-ivanov jonatan-ivanov modified the milestones: 1.10.x, 1.10.13 Oct 24, 2023
@antechrestos
Copy link
Contributor Author

@jonatan-ivanov of course. I will do it by tomorrow with your suggestion.
Thank you

@antechrestos antechrestos changed the base branch from main to 1.10.x October 24, 2023 20:31
@antechrestos
Copy link
Contributor Author

@jonatan-ivanov I made the following change

  • applied suggestion
  • change target branch (with rebase)

Cheers

@jonatan-ivanov jonatan-ivanov merged commit 003c910 into micrometer-metrics:1.10.x Oct 25, 2023
5 checks passed
@jonatan-ivanov
Copy link
Member

Thank you!

@jonatan-ivanov jonatan-ivanov added the registry: otlp OpenTelemetry Protocol (OTLP) registry-related label Oct 25, 2023
@antechrestos
Copy link
Contributor Author

@jonatan-ivanov do i need to report the change on other branches? Which ones? I spotted that code was not really the same...

@antechrestos antechrestos deleted the fixes/add-warning-when-publication-of-metrics-fails branch October 25, 2023 21:13
@jonatan-ivanov
Copy link
Member

Nope, once it is merged to a maintenance branch the person who merged it should forward merge it to the other supported branches too. I've already taken care of forward merging and resolving the merge conflicts (as you noticed, the code changed a bit). This change is now available on 1.10.x, 1.11.x, and main, see 538ba6a and 32fe2c5

You can try out any latest snapshot versions, e.g.: 1.12.0-SNAPSHOT, it should contain your changes, thank you again for fixing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A general bug registry: otlp OpenTelemetry Protocol (OTLP) registry-related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants