[ENH] Latency histograms for get/insert/remove/clear of cache. #3018
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
I think metrics should automatically appear in Grafana with no config changes, let me know if that isn't the case. edit: I think that actually might just be set up for Go services? but should be easy to update the env var for the Rust services too |
| #[tracing::instrument(skip(self, key, value))] | ||
| async fn insert(&self, key: K, value: V) { | ||
| let _stopwatch = Stopwatch::new(&self.insert_latency); |
There was a problem hiding this comment.
curious why this is measured separately instead of looking at the span duration?
|
It's not the case. You can set the endpoint to either jaeger or otlp-collector, but not both. Because two endpoints only matters for tilt, I'm fine not having these metrics show up locally. I confirmed that they do show up locally, though, if the address is right. In production, we have honeycomb, which seems to have just one endpoint for both. |
| @@ -96,4 +161,13 @@ pub(crate) fn init_otel_tracing(service_name: &String, otel_endpoint: &String) { | |||
|
|
|||
| prev_hook(panic_info); | |||
| })); | |||
| let exporter = opentelemetry_otlp::new_exporter() | |||
There was a problem hiding this comment.
for my learning what is this new exporter and provider for?
There was a problem hiding this comment.
Metrics. They get installed as the exporter and provider for the global metrics. Do you see a way to reuse the other exporter? I'd like that, too.
hmm we should be able to just set |
## Description of changes * This wires up the cache to give us latency metrics for all operations. ## Test plan I manually verified that the traces are exported if I change the otel_endpoint to otel-collector:4317 instead of jaeger:4317 - [ X] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust
## Description of changes *Summarize the changes made by this PR.* - Improvements & Bug fixes - Send telemetry to otel_collector not jaeger. Fixing the left over in #3018 deemed out of scope by @codetheweb in #3018 (comment) - New functionality - None ## Test plan *How are these changes tested?* - [x] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes None
Description of changes
Test plan
I manually verified that the traces are exported if I change the otel_endpoint to otel-collector:4317 instead of jaeger:4317
pytestfor python,yarn testfor js,cargo testfor rust