opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

[prototype] feat(api): add wrapMeter() for experimental metrics API features

Open pichlermarc opened this issue 1 year ago • 2 comments

Which problem is this PR solving?

This is based on @clintonb's work on adding a createGauge() API.

We did not have a way to introduce experimental API features for the metrics API. This PR implements a function wrapMeter available via @opentelemetry/api/experimental that allows users to use experimental features on a Meter.

It falls back to a no-op should trying to create the Instrument on the underlying Meter fail (if the function is not present, for instance).

Type of change

Please delete options that are not relevant.

  • [x] New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • [ ] Unit tests

pichlermarc avatar Apr 10 '24 15:04 pichlermarc

Codecov Report

Attention: Patch coverage is 96.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 92.90%. Comparing base (2610122) to head (b630553). Report is 47 commits behind head on main.

:exclamation: Current head b630553 differs from pull request most recent head abb2e36

Please upload reports for the commit abb2e36 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4622      +/-   ##
==========================================
+ Coverage   90.77%   92.90%   +2.12%     
==========================================
  Files          90      292     +202     
  Lines        1930     7919    +5989     
  Branches      417     1667    +1250     
==========================================
+ Hits         1752     7357    +5605     
- Misses        178      562     +384     
Files Coverage Δ
api/src/experimental/index.ts 100.00% <100.00%> (ø)
api/src/experimental/metrics/ExperimentalMeter.ts 100.00% <100.00%> (ø)
api/src/metrics/NoopMeter.ts 97.36% <ø> (ø)
api/src/experimental/metrics/Gauge.ts 75.00% <75.00%> (ø)

... and 201 files with indirect coverage changes

codecov[bot] avatar Apr 18 '24 12:04 codecov[bot]

I've been fighting webpack for a while now on this problem. I'm pretty sure we're doing something wrong with our entrypoint exports, but I just cannot figure out what it is.

Edit: I wonder if that's because we're using karma-webpack and that's using webpack 4. Maybe it does not support it yet? Edit 2: No can't be it because we only have webpack 5 installed, I wonder why npm does not compiain as the peer-dependency range of ^4.0.0 for webpack is not matched when installing karma-webpack. Edit 3: It's the old version of karma-webpack that's the problem, now we have a few choices, all of which are kind of bad/not-easy to do:

  • update to karma-webpack 5.0.1, break tests for all macOS users (not sure if that's actually what would happen, will try to find someone to run the changes from #4648 on macOS)
  • switch to @web/test-runner and rollup:
    • con: it's a lot of work
    • con: does not work well in WebStorm as there's no plugin for it, but I'll survive that, no biggie.
  • contribute a fix to karma-webpack
    • we'll have to hope that it get's released soon Edit 4: looks like this is actually fixed in the lastest release of karma-webpack, so not an issue we'll be able to proceed here once #4648 is merged.

pichlermarc avatar Apr 18 '24 13:04 pichlermarc