Add exemplar support #51
Conversation
actualwitch
force-pushed
the
add-exemplars
branch
3 times, most recently
from
20ca9fa to
f1fbf1d
Compare
actualwitch
force-pushed
the
add-exemplars
branch
from
f1fbf1d to
efe0622
Compare
| from ..objectives import Objective | ||
|
|
||
|
|
||
| prometheus_client.REGISTRY.unregister(prometheus_client.GC_COLLECTOR) |
There was a problem hiding this comment.
could you add a comment explaining these unregister calls? was this something we should've been doing all along?
not sure if this is what you read, but this thread seemed relevant
There was a problem hiding this comment.
my bad, i set that when i was debugging exemplars but forgor 💀 to remove it. we probably shouldn't set these in autometrics.
README.md
Outdated
| @@ -130,6 +134,15 @@ The `version` is read from the `AUTOMETRICS_VERSION` environment variable, and t | |||
|
|
|||
| This follows the method outlined in [Exposing the software version to Prometheus](https://www.robustperception.io/exposing-the-software-version-to-prometheus/). | |||
|
|
|||
| ## Exemplars | |||
|
|
|||
| > **NOTE** - As of writing, aren't supported by the default tracker (`AUTOMETRICS_TRACKER=OPEN_TELEMETRY`). | |||
There was a problem hiding this comment.
I think the word exemplars is missing here:
As of writing, exemplars aren't supported...
There was a problem hiding this comment.
I was also thinking that maybe it's better to first explain what exemplars are before mentioning the limitations/caveats
There was a problem hiding this comment.
good eye! indeed, the word was missing - fixed now
There was a problem hiding this comment.
i added the note above for consistency with the previous paragraph, it would look like this:
i think since it's quoted people can just look below, but the quoted part is quite big. if you think it's not obvious i will move the note below
There was a problem hiding this comment.
It makes more sense to me to explain first and then mention what the caveats are especially if people aren't super familiar with what exemplars are even if you can skip, notes are typically important so for me (personally) I would still try to read it first., but that's just anecdotal (and also I don't feel that strongly about it) so i'm fine with leaving it the way you have it now.
There was a problem hiding this comment.
i ended up moving a bunch of text from the quoted part below, i think this should be fine now
| self.__count(function, module, caller, objective, result) | ||
| self.__histogram(function, module, start_time, objective) | ||
| exemplar = None | ||
| # Currently, exemplars are only supported by prometheus-client |
There was a problem hiding this comment.
I think it might be good to keep an issue open for this. So we can refer to it from our source code and also have a place to talk about how to deal/resolve this
There was a problem hiding this comment.
we actually have one! i added a backlink there and also linked the issue from code
actualwitch
force-pushed
the
add-exemplars
branch
from
efe0622 to
4ae9fb5
Compare
|
i also updated the example to show we track exemplars for all autometrized functions inside the span, a la rust version |
actualwitch
force-pushed
the
add-exemplars
branch
7 times, most recently
from
016c8fe to
4e83474
Compare
actualwitch
force-pushed
the
add-exemplars
branch
from
4e83474 to
73e2037
Compare
No description provided.