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

chore!: split metrics into its own api package #1797

Merged
merged 14 commits into from
Jan 15, 2021

Conversation

dyladan
Copy link
Member

@dyladan dyladan commented Jan 7, 2021

Create new @opentelemetry/metrics-api package

  • update api readme
  • update metrics api readme
  • new metrics api package is a copy of the api package with non-metrics items removed
    • global utils is duplicated for now (avoids depending on api internals from metrics api)
    • global this is duplicated for now (same reason)
  • remove all references to metrics from standard api package
  • update imports in dependent modules
  • no functionality changes of any kind
  • ts references updated

Follow up:

  • Move API and context-base out of lerna (new repo?)
  • Shared (stable) api-common package for shared things like logging interface?
    • Currently api-metrics just depends on api to get these things.

@codecov
Copy link

codecov bot commented Jan 7, 2021

Codecov Report

Merging #1797 (b165c79) into master (342b168) will increase coverage by 0.49%.
The diff coverage is 94.28%.

@@            Coverage Diff             @@
##           master    #1797      +/-   ##
==========================================
+ Coverage   92.12%   92.61%   +0.49%     
==========================================
  Files         163      174      +11     
  Lines        5484     6042     +558     
  Branches     1177     1283     +106     
==========================================
+ Hits         5052     5596     +544     
- Misses        432      446      +14     
Impacted Files Coverage Δ
...opentelemetry-api-metrics/src/NoopMeterProvider.ts 100.00% <ø> (ø)
...ages/opentelemetry-api-metrics/src/types/Metric.ts 100.00% <ø> (ø)
packages/opentelemetry-api/src/api/global-utils.ts 100.00% <ø> (ø)
...ry-exporter-prometheus/src/PrometheusSerializer.ts 98.01% <ø> (ø)
...entelemetry-instrumentation/src/autoLoaderUtils.ts 100.00% <ø> (ø)
...es/opentelemetry-metrics/src/BaseObserverMetric.ts 100.00% <ø> (ø)
...ackages/opentelemetry-metrics/src/BatchObserver.ts 100.00% <ø> (ø)
...s/opentelemetry-metrics/src/BatchObserverResult.ts 100.00% <ø> (ø)
...ackages/opentelemetry-metrics/src/CounterMetric.ts 100.00% <ø> (ø)
...ackages/opentelemetry-metrics/src/MeterProvider.ts 89.47% <ø> (ø)
... and 32 more

@dyladan
Copy link
Member Author

dyladan commented Jan 11, 2021

@open-telemetry/javascript-approvers this is extremely important for the API release so please review.

Copy link
Member

@obecny obecny left a comment

Choose a reason for hiding this comment

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

overall lgtm, many changes :).
Can we have a new folder "types" inside api metrics and then move there all interfaces ? Currently we have interfaces and noop Implementations together in main folder, this could help organise stuff WDYT ?

packages/opentelemetry-api-metrics/README.md Outdated Show resolved Hide resolved
packages/opentelemetry-api-metrics/README.md Outdated Show resolved Hide resolved
packages/opentelemetry-api-metrics/test/api/global.test.ts Outdated Show resolved Hide resolved
@dyladan
Copy link
Member Author

dyladan commented Jan 13, 2021

@obecny made all requested changes

Copy link
Member

@obecny obecny left a comment

Choose a reason for hiding this comment

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

lgtm

@dyladan dyladan added enhancement New feature or request breaking labels Jan 15, 2021
@dyladan dyladan changed the title chore: split metrics into its own api package chore!: split metrics into its own api package Jan 15, 2021
@dyladan dyladan merged commit 5d1b4ee into open-telemetry:master Jan 15, 2021
@dyladan dyladan deleted the split-metrics branch January 15, 2021 13:55
dyladan added a commit to dyladan/opentelemetry-js that referenced this pull request Sep 9, 2022
dyladan added a commit to dyladan/opentelemetry-js that referenced this pull request Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants