fix(monitoring-exporter): use correct value type for histogram/distribution metrics

[jbaldassari]

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

• Make sure to open an issue as a bug/issue/feature before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
• Ensure the tests and linter pass
• Code coverage does not decrease (if any source code was changed)
• Appropriate docs were updated (if necessary)

Fixes #473

The package-lock.json file was not in sync with package.json when I went to make this change. npm ci reported:

npm ERR! code EUSAGE
npm ERR! 
npm ERR! `npm ci` can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with `npm install` before continuing.
npm ERR! 
npm ERR! Missing: @google-cloud/opentelemetry-resource-util@1.2.0 from lock file

I had to npm i to resolve the issue, which is the reason for the package-lock.json changes. I can revert this change if you'd like, but it seems like the lock file was invalid, so that should probably be fixed.

[aabmass]

/gcbrun

@jbaldassari the package-lock.json changes seem to just be re-indenting the file. I've seen this kind of noise sometimes when the npm versions generating the lockfile are different. If you revert the change, does the CI fail?

[codecov]

Codecov Report

Merging #474 (a749572) into main (002b61a) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head a749572 differs from pull request most recent head 08d059f. Consider uploading reports for the commit 08d059f to get more accurate results

@@            Coverage Diff             @@
##             main     #474      +/-   ##
==========================================
+ Coverage   95.62%   95.64%   +0.01%     
==========================================
  Files          16       16              
  Lines         549      551       +2     
  Branches      102      103       +1     
==========================================
+ Hits          525      527       +2     
  Misses         24       24              

Impacted Files	Coverage Δ
...lemetry-cloud-monitoring-exporter/src/transform.ts	80.95% <100.00%> (+0.46%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[jbaldassari]

@aabmass I noticed the indentation changes as well, which is odd, but the other change is that the version on main does not contain an entry for @google-cloud/opentelemetry-resource-util despite it being in package.json. npm ci fails on the main version with this error:

$ npm ci
npm WARN prepare removing existing node_modules/ before installation
npm ERR! cipm can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with `npm install` before continuing.
npm ERR! 
npm ERR! 
npm ERR! Missing: @google-cloud/opentelemetry-resource-util@^1.2.0
npm ERR! 

Again, I'm happy to revert this change if you'd prefer to deal with it internally. I just had to get it fixed so that I could install node_modules in my checkout. Anyway let me know what you'd like me to do.

[aabmass]

@jbaldassari I checked out your PR, checkout the package-lock.json from main, and ran npm install. It didn't output any changes. Did you run npm install from within the package directory or from the repo root (we are using lerna, so you have to run it from the repo root to link the monorepo).

I'd recommend just revert that file and update the PR. I think the CI will pass.

[aabmass]

/gcbrun

[jbaldassari]

Thanks for the tip @aabmass . That was it. I was running it from the package directory. Running it from the repo root works. Well, I should say it does not update package-lock.json. I do get several failures when I run npm i from the root, such as this one from packages/opentelemetry-cloud-monitoring-exporter (and many more like it):

src/monitoring.ts:19:8 - error TS2307: Cannot find module '@opentelemetry/sdk-metrics' or its corresponding type declarations.

19 } from '@opentelemetry/sdk-metrics';

I noticed that opentelemetry-cloud-monitoring-exporter does not depend on @opentelemetry/sdk-metrics, which I thought was strange since it imports classes/interfaces from that package. I'm happy to just chalk this up as me not knowing much about lerna though 🤷

I've updated the PR to revert the package-lock.json changes.

[aabmass]

LGTM thanks for the PR 😄

[jbaldassari]

Thank you for taking a look so quickly!

[aabmass]

I noticed that opentelemetry-cloud-monitoring-exporter does not depend on @opentelemetry/sdk-metrics, which I thought was strange since it imports classes/interfaces from that package. I'm happy to just chalk this up as me not knowing much about lerna though 🤷

It's added as a peer dependency as it is a "plugin" to the SDK

opentelemetry-operations-js/packages/opentelemetry-cloud-monitoring-exporter/package.json

Line 77 in 002b61a

"@opentelemetry/sdk-metrics": "^1.0.0"

For build purposes, I believe newer versions of npm will automatically install peerDependencies. But you're right, I should add a second entry to devDependencies to pull it in just in case.