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

feat(mysql): Metrics for mysql #1220

Merged
merged 81 commits into from
Jan 4, 2023

Conversation

haddasbronfman
Copy link
Member

@haddasbronfman haddasbronfman commented Oct 7, 2022

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

@haddasbronfman haddasbronfman changed the title feat(mysql): started working on mysql metrics feat(mysql): Metrics for mysql Oct 7, 2022
@codecov
Copy link

codecov bot commented Oct 7, 2022

Codecov Report

Merging #1220 (529d784) into main (72e961c) will decrease coverage by 0.14%.
The diff coverage is 94.82%.

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     
Impacted Files Coverage Δ
...metry-instrumentation-mysql/src/instrumentation.ts 93.60% <93.75%> (ø)
...e/opentelemetry-instrumentation-mysql/src/utils.ts 100.00% <100.00%> (ø)
...tapackages/auto-instrumentations-node/src/utils.ts 98.21% <0.00%> (ø)

Copy link
Member

@pichlermarc pichlermarc left a 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 🙂

Copy link
Member

@dyladan dyladan left a 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.
Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member

@pichlermarc pichlermarc left a 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 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metrics instrumentation mysql
8 participants