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

Micrometer bridge: interpret no SLO config as no buckets advice #8856

Merged
merged 1 commit into from
Jul 5, 2023

Conversation

mateuszrzeszutek
Copy link
Member

Resolves #8841 (?)

cc @zeitlinger

This implements my idea from the issue:

By default Micrometer Timers have no buckets at all, unless you configure serviceLevelObjectives via the builder. Perhaps what we should do is preserve that behavior in the bridge as well -- and pass an empty bucket list if the user has not explicitly configured them via Micrometer API. At least it would be consistent with how "plain" Micrometer behaves.

@mateuszrzeszutek mateuszrzeszutek requested review from zeitlinger and a team July 3, 2023 09:16
@zeitlinger
Copy link
Member

Thanks @mateuszrzeszutek for taking the time to explain - I also approve 😄

@mateuszrzeszutek
Copy link
Member Author

mateuszrzeszutek commented Jul 4, 2023

You're welcome! I definitely agree that the number of layers and components here might get confusing

@trask
Copy link
Member

trask commented Jul 5, 2023

I also approve 😄

❤️

even though you're not an official approver, you can still hit the approve button, we definitely factor in non-green checkmarks when merging

@trask trask merged commit d875d99 into open-telemetry:main Jul 5, 2023
@mateuszrzeszutek mateuszrzeszutek deleted the micrometer-no-buckets branch July 5, 2023 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Micrometer metrics bridge] Default bucket boundaries assume milliseconds base unit
4 participants