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.

Sign up for GitHub

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

Jump to bottom

Prepare to move metrics code to separate subpackage #1316

Merged
merged 4 commits into from
Nov 11, 2020
Merged

Prepare to move metrics code to separate subpackage #1316

merged 4 commits into from
Nov 11, 2020

Conversation

krnowak
Copy link
Member

@krnowak krnowak commented Nov 9, 2020 •
edited by MrAlias
Loading

Metric is quite a hefty amount of code to be moved around, so split the move by moving the registry and number code into the metric subpackage. This PR comes as a part of solving #1303.

Resolves #1287

@codecov
Copy link

codecov bot commented Nov 9, 2020 •
edited
Loading

Codecov Report

Merging #1316 (2ec9b4b) into master (f9984f2) will decrease coverage by 0.1%.
The diff coverage is 68.2%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #1316     +/-   ##
========================================
- Coverage    77.4%   77.3%   -0.2%     
========================================
  Files         122     122             
  Lines        5975    5978      +3     
========================================
- Hits         4626    4622      -4     
- Misses       1099    1107      +8     
+ Partials      250     249      -1
Impacted Files Coverage Δ
exporters/metric/prometheus/prometheus.go 57.6% <0.0%> (ø)
global/internal/meter.go 90.3% <ø> (ø)
metric/registry/registry.go 92.1% <ø> (ø)
metric_noop.go 0.0% <ø> (ø)
metric_sdkapi.go 100.0% <ø> (ø)
sdk/export/metric/aggregation/aggregation.go 100.0% <ø> (ø)
sdk/export/metric/metric.go 97.3% <ø> (ø)
sdk/export/metric/metrictest/test.go 19.3% <ø> (ø)
sdk/metric/aggregator/histogram/histogram.go 91.1% <ø> (ø)
sdk/metric/aggregator/lastvalue/lastvalue.go 78.5% <ø> (ø)
... and 18 more

MrAlias
MrAlias approved these changes Nov 9, 2020
View reviewed changes
metric/number/number.go Outdated Show resolved Hide resolved
@krnowak
Copy link
Member Author

krnowak commented Nov 10, 2020

Updated and rebased. The CI failure seems to be a flake, not related to my changes.

@MrAlias MrAlias added area:metrics Part of OpenTelemetry Metrics pkg:API Related to an API package priority:p1 labels Nov 11, 2020
@MrAlias MrAlias added this to the RC1 milestone Nov 11, 2020
@MrAlias MrAlias merged commit 386331a into open-telemetry:master Nov 11, 2020
@krnowak krnowak deleted the new-api-2 branch November 11, 2020 15:52
@MrAlias MrAlias mentioned this pull request Nov 20, 2020
Merged
AzfaarQureshi pushed a commit to open-o11y/opentelemetry-go that referenced this pull request Dec 3, 2020
@krnowak
Prepare to move metrics code to separate subpackage (open-telemetry#1316
189e1bc 
)

* Move registry package under metric

* Move Number type to the metric/number subpackage

This also renames NumberKind type to Kind.

* Update changelog

* Drop outdated comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XSAM XSAM XSAM approved these changes

@MrAlias MrAlias MrAlias approved these changes

@Aneurysm9 Aneurysm9 Awaiting requested review from Aneurysm9

@evantorrie evantorrie Awaiting requested review from evantorrie

@jmacd jmacd Awaiting requested review from jmacd

@lizthegrey lizthegrey Awaiting requested review from lizthegrey

Assignees
No one assigned
Labels
area:metrics Part of OpenTelemetry Metrics pkg:API Related to an API package
Projects
None yet
Milestone
RC1
Development

Successfully merging this pull request may close these issues.

Consider moving Number into its own package
3 participants
@krnowak @MrAlias @XSAM