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

Rename "handle" to "bound metric instrument" #470

Merged
merged 11 commits into from
Mar 10, 2020
Merged

Rename "handle" to "bound metric instrument" #470

merged 11 commits into from
Mar 10, 2020

Conversation

lzchen
Copy link
Contributor

@lzchen lzchen commented Mar 9, 2020

Solves [#468]

@lzchen lzchen requested a review from a team March 9, 2020 19:38
@codecov-io
Copy link

codecov-io commented Mar 9, 2020 •
edited
Loading

Codecov Report

Merging #470 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #470      +/-   ##
==========================================
- Coverage   88.98%   88.97%   -0.02%     
==========================================
  Files          43       43              
  Lines        2216     2213       -3     
  Branches      248      248              
==========================================
- Hits         1972     1969       -3     
  Misses        173      173              
  Partials       71       71
Impacted Files Coverage Δ
...elemetry-api/src/opentelemetry/metrics/__init__.py 85.18% <100%> (ø) ⬆️
...etry-sdk/src/opentelemetry/sdk/metrics/__init__.py 94.4% <100%> (-0.11%) ⬇️
...emetry-sdk/src/opentelemetry/sdk/trace/__init__.py 92.51% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7d9b15...8710493. Read the comment docs.

@lzchen lzchen added metrics needs reviewers PRs with this label are ready for review and needs people to review to move forward. labels Mar 9, 2020
c24t
c24t approved these changes Mar 10, 2020
View reviewed changes
Copy link
Member

@c24t c24t left a comment

Choose a reason for hiding this comment

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

LGTM!

mauriciovasquezbernal
mauriciovasquezbernal approved these changes Mar 10, 2020
View reviewed changes
Copy link
Member

@mauriciovasquezbernal mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

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

LGTM.

I see that there are some "bound metric" references around, is that intended?

opentelemetry-api/src/opentelemetry/metrics/__init__.py Outdated Show resolved Hide resolved
opentelemetry-api/src/opentelemetry/metrics/__init__.py Outdated Show resolved Hide resolved
opentelemetry-api/src/opentelemetry/metrics/__init__.py Outdated Show resolved Hide resolved
examples/metrics/record.py Outdated Show resolved Hide resolved
opentelemetry-api/src/opentelemetry/metrics/__init__.py Outdated Show resolved Hide resolved
toumorokoshi
toumorokoshi suggested changes Mar 10, 2020
View reviewed changes
Copy link
Member

@toumorokoshi toumorokoshi left a comment

Choose a reason for hiding this comment

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

The renames look good! But I think @mauriciovasquezbernal calls out some critical inconsistencies in the docs.

Since it's a quick change, I think they should be addressed before we merge.

toumorokoshi
toumorokoshi approved these changes Mar 10, 2020
View reviewed changes
Copy link
Member

@toumorokoshi toumorokoshi left a comment

Choose a reason for hiding this comment

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

thank!

@toumorokoshi toumorokoshi merged commit 4b6a52d into open-telemetry:master Mar 10, 2020
@lzchen lzchen deleted the bound branch March 10, 2020 17:59
@mauriciovasquezbernal mauriciovasquezbernal mentioned this pull request Mar 10, 2020
Closed
toumorokoshi added a commit to toumorokoshi/opentelemetry-python that referenced this pull request Mar 16, 2020
@toumorokoshi
Adding changelog entries for the 0.5b0 release
664b86d 
Total Changelog:

Documentations has been significantly overhauled, including:

* a getting started guide
* examples has been consolidated to an docs/examples folder
* several minor improvements to the examples in each extension's readme.

- Adding Correlation Context API and propagator
  ([open-telemetry#471](open-telemetry#471))
- Adding a global configuration module to simplify setting and getting
  globals
  ([open-telemetry#466](open-telemetry#466))
- Rename metric handle to bound metric
  ([open-telemetry#470](open-telemetry#470))
- Moving resources to sdk
  ([open-telemetry#464](open-telemetry#464))
- Implementing propagators to API to use context
  ([open-telemetry#446](open-telemetry#446))
- Implement observer instrument for metrics
  ([open-telemetry#425](open-telemetry#425))
- Adding named meters, removing batchers
  ([open-telemetry#431](open-telemetry#431))
- Renaming TraceOptions to TraceFlags
  ([open-telemetry#450](open-telemetry#450))
- Renaming TracerSource to TraceProvider
  ([open-telemetry#441](open-telemetry#441))

- Adding Correlation Context SDK and propagator
  ([open-telemetry#471](open-telemetry#471))
- Adding OT Collector metrics exporter
  ([open-telemetry#454](open-telemetry#454))
- Improve validation of attributes
  ([open-telemetry#460](open-telemetry#460))
- Re-raise errors caught in opentelemetry.sdk.trace.Tracer.use_span()
  (open-telemetry#469)
  ([open-telemetry#469](open-telemetry#469))
- Adding named meters, removing batchers
  ([open-telemetry#431](open-telemetry#431))
- Make counter and MinMaxSumCount aggregators thread safe
  ([open-telemetry#439](open-telemetry#439))

- Initial release. Support is included for both trace and metrics.
@toumorokoshi toumorokoshi mentioned this pull request Mar 16, 2020
Merged
@lzchen lzchen changed the title Rename "handle" to "bound metric" Rename "handle" to "bound metric instrument" Mar 16, 2020
c24t pushed a commit to c24t/opentelemetry-python that referenced this pull request Mar 16, 2020
@toumorokoshi @c24t
Adding changelog entries for the 0.5b0 release
8004571 
Total Changelog:

Documentations has been significantly overhauled, including:

* a getting started guide
* examples has been consolidated to an docs/examples folder
* several minor improvements to the examples in each extension's readme.

- Adding Correlation Context API and propagator
  ([open-telemetry#471](open-telemetry#471))
- Adding a global configuration module to simplify setting and getting
  globals
  ([open-telemetry#466](open-telemetry#466))
- Rename metric handle to bound metric
  ([open-telemetry#470](open-telemetry#470))
- Moving resources to sdk
  ([open-telemetry#464](open-telemetry#464))
- Implementing propagators to API to use context
  ([open-telemetry#446](open-telemetry#446))
- Implement observer instrument for metrics
  ([open-telemetry#425](open-telemetry#425))
- Adding named meters, removing batchers
  ([open-telemetry#431](open-telemetry#431))
- Renaming TraceOptions to TraceFlags
  ([open-telemetry#450](open-telemetry#450))
- Renaming TracerSource to TraceProvider
  ([open-telemetry#441](open-telemetry#441))

- Adding Correlation Context SDK and propagator
  ([open-telemetry#471](open-telemetry#471))
- Adding OT Collector metrics exporter
  ([open-telemetry#454](open-telemetry#454))
- Improve validation of attributes
  ([open-telemetry#460](open-telemetry#460))
- Re-raise errors caught in opentelemetry.sdk.trace.Tracer.use_span()
  (open-telemetry#469)
  ([open-telemetry#469](open-telemetry#469))
- Adding named meters, removing batchers
  ([open-telemetry#431](open-telemetry#431))
- Make counter and MinMaxSumCount aggregators thread safe
  ([open-telemetry#439](open-telemetry#439))

- Initial release. Support is included for both trace and metrics.
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this pull request Nov 1, 2020
@dyladan @mayurkale22
chore: remove examples from lerna packages (open-telemetry#470)
df58228 
Examples fail to run if they are installed with the regular boostrap process.
Removing them from packages allows the user to `npm install` them
regularly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@c24t c24t c24t approved these changes

@mauriciovasquezbernal mauriciovasquezbernal mauriciovasquezbernal approved these changes

@toumorokoshi toumorokoshi toumorokoshi approved these changes

Assignees
No one assigned
Labels
metrics needs reviewers PRs with this label are ready for review and needs people to review to move forward.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lzchen @codecov-io @toumorokoshi @mauriciovasquezbernal @c24t