Skip to content

feat: Add prometheus exporter #483

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

dyladan
Copy link
Member

@dyladan dyladan commented Nov 4, 2019

Which problem is this PR solving?

With #482 this now provides an end-to-end metrics story

Short description of the changes

Adds a metrics exporter for the prometheus backend. In its current state, the exporter maintains an internal state with the values of all exported metrics, which is then returned when an endpoint is called by Prometheus.

@codecov-io
Copy link

codecov-io commented Nov 4, 2019

Codecov Report

Attention: Patch coverage is 96.38158% with 11 lines in your changes missing coverage. Please review.

Project coverage is 92.60%. Comparing base (39732fa) to head (20efbba).

Files with missing lines Patch % Lines
...pentelemetry-exporter-prometheus/src/prometheus.ts 91.83% 8 Missing ⚠️
...emetry-exporter-prometheus/test/prometheus.test.ts 98.54% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #483      +/-   ##
==========================================
+ Coverage   92.03%   92.60%   +0.57%     
==========================================
  Files         138      145       +7     
  Lines        6264     7114     +850     
  Branches      527      626      +99     
==========================================
+ Hits         5765     6588     +823     
- Misses        499      526      +27     
Files with missing lines Coverage Δ
...emetry-exporter-prometheus/test/prometheus.test.ts 98.54% <98.54%> (ø)
...pentelemetry-exporter-prometheus/src/prometheus.ts 91.83% <91.83%> (ø)

... and 31 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@mayurkale22 mayurkale22 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 a great start, added a few comments in the first pass.

Co-Authored-By: Mayur Kale <[email protected]>
Copy link
Member

@mayurkale22 mayurkale22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @dyladan for this PR and for addressing my feedback! LGTM!

* @param name name to be sanitized
*/
private _sanitizePrometheusMetricName(name: string): string {
return name.replace(this._invalidCharacterRegex, '_'); // replace all invalid characters with '_'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible this could cause duplicates? Should we write a warning in that case (could be a follow up task)?

Copy link
Member Author

@dyladan dyladan Nov 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could replace period with __DOT__ or similar

@mayurkale22
Copy link
Member

I managed to run the end to end example using prometheus exporter, looks like everything is working fine. I tried creating and updating simple counter and gauge metrics every 2 secs.

/metrics endpoint:
Screen Shot 2019-11-06 at 5 19 03 PM
On scraper:
Screen Shot 2019-11-06 at 5 20 10 PM

@mayurkale22 mayurkale22 merged commit 460d986 into open-telemetry:master Nov 11, 2019
@Flarna Flarna deleted the add-prometheus-exporter-dan branch December 13, 2019 20:58
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Dec 15, 2023
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this pull request Dec 15, 2023
* feat: add responseHook config to redis instrumentation

* fix: lint fix

* feat: add responseHook config to redis instrumentation

* fix: lint fix

* chore(deps): update all non-major dependencies (open-telemetry#483)

* chore: generalize the instrumentation file name (open-telemetry#479)

* feat: add responseHook config to graphql instrumentation

* Apply suggestions from code review

Co-authored-by: Bartlomiej Obecny <[email protected]>

* test: add a test for a responseHook that isn't a function

* fix: lint fix

Co-authored-by: WhiteSource Renovate <[email protected]>
Co-authored-by: Rauno Viskus <[email protected]>
Co-authored-by: Bartlomiej Obecny <[email protected]>
martinkuba pushed a commit to martinkuba/opentelemetry-js that referenced this pull request Mar 13, 2024
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.

6 participants