-
Notifications
You must be signed in to change notification settings - Fork 515
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
feat(mysql): Metrics for mysql #1220
feat(mysql): Metrics for mysql #1220
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1220 +/- ##
==========================================
- Coverage 96.08% 95.93% -0.15%
==========================================
Files 14 17 +3
Lines 893 1157 +264
Branches 191 231 +40
==========================================
+ Hits 858 1110 +252
- Misses 35 47 +12
|
…emetry-js-contrib into add-mysql-metrics
…emetry-js-contrib into add-mysql-metrics
…emetry-js-contrib into add-mysql-metrics
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.
This is looking great already 🙂
Just a few nits + comments from my side.
Thank you for taking the time to work on this 🙂
plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
…emetry-js-contrib into add-mysql-metrics
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.
LGTM. Minor nit on the readme/docs but the impl looks good
@@ -1,6 +1,6 @@ | |||
# Overview | |||
|
|||
OpenTelemetry MySQL Instrumentation allows the user to automatically collect trace data and export them to the backend of choice (we can use Zipkin or Jaeger for this example), to give observability to distributed systems. | |||
OpenTelemetry MySQL Instrumentation allows the user to automatically collect trace data and metrics (currently only the number of connections is supported) and export them to the backend of choice (we can use Zipkin, Jaeger or Grafana for this example), to give observability to distributed systems. |
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.
People may not notice this in the prose when they are using or when they modify the instrumentation. I would just leave it as "trace data and metrics" and link to a list of supported metrics in another heading.
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.
thanks. I'll fix that.
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.
Great work! Thank you for adding this 🙂
Which problem is this PR solving?
fixes #1176
Short description of the changes
Add the metric
db.client.connections.usage
to mysql according to this: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/database-metrics.md#connection-pools