-
Notifications
You must be signed in to change notification settings - Fork 15
feat(telemetry): track tracer and telemetry usage #209
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
Conversation
Introduce additional telemetry metrics to gain deeper insights into our tracer usage.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #209 +/- ##
==========================================
- Coverage 87.09% 87.01% -0.08%
==========================================
Files 80 80
Lines 5084 5138 +54
==========================================
+ Hits 4428 4471 +43
- Misses 656 667 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| std::move(body), std::move(on_response), | ||
| std::move(on_error), clock_().tick + request_timeout_); | ||
| if (auto* error = post_result.if_error()) { | ||
| // NOTE(@dmehala): `technical` is a better kind of errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this comment mean?
|
|
||
| void Telemetry::log_error(std::string message) { | ||
| if (!config_.report_logs) return; | ||
| increment_counter(internal_metrics::logs_created, {"level:error"}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason not to do these inside the log function itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because increment_counter(internal_metrics::logs_created, {"level:error"}); is more readable than increment_counter(internal_metrics::logs_created, {std::string("level:) + to_string(level)"});
| }; | ||
|
|
||
| send_telemetry("app-started", app_started()); | ||
| http_client_->drain(clock_().tick + 2s); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing this is here to make sure app-started is the absolute first. What is the problem if it is not the first?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per telemetry doc:
Telemetry Lifecycle
The first event we should receive is app-started. [...]
src/datadog/telemetry_metrics.cpp
Outdated
| const telemetry::Distribution trace_chunk_size_bytes = { | ||
| "trace_chunk_serialization.bytes", "tracers", true}; | ||
|
|
||
| const telemetry::Distribution trace_chunk_size_ms = { | ||
| "trace_chunk_serialization.ms", "tracers", true}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: size_* -> serialization_*?
BenchmarksBenchmark execution time: 2025-04-30 12:35:42 Comparing candidate commit be713f7 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. |
Description
Introduce additional telemetry metrics to gain deeper insights into our tracer usage.