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

Metrics API #137

Merged
merged 11 commits into from
Jun 20, 2019
Merged

Metrics API #137

merged 11 commits into from
Jun 20, 2019

Conversation

SergeyKanzhelev
Copy link
Member

@SergeyKanzhelev SergeyKanzhelev commented Jun 18, 2019 •
edited
Loading

Fixes #40

@SergeyKanzhelev SergeyKanzhelev changed the title Metrics API [WIP] Metrics API Jun 18, 2019
@reyang
Copy link
Member

reyang commented Jun 19, 2019

@lzchen please take a look, thanks.

@SergeyKanzhelev SergeyKanzhelev changed the title [WIP] Metrics API Metrics API Jun 19, 2019
lzchen
lzchen requested changes Jun 19, 2019
View reviewed changes
specification/metrics-api.md Outdated Show resolved Hide resolved
specification/metrics-api.md Outdated Show resolved Hide resolved
specification/metrics-api.md Show resolved Hide resolved
@iredelmeier iredelmeier requested a review from jmacd June 19, 2019 18:35
@lzchen lzchen mentioned this pull request Jun 19, 2019
Closed
tedsuo
tedsuo approved these changes Jun 20, 2019
View reviewed changes
Copy link
Contributor

@tedsuo tedsuo left a comment

Choose a reason for hiding this comment

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

Generally, this LGTM, as a port of what the Java API is currently doing. If you are going to leave in the TODO's for now, then I believe it is fine to commit this and iterate in future PRs.

songy23
songy23 approved these changes Jun 20, 2019
View reviewed changes
Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

A few minor comments, otherwise LGTM.

specification/metrics-api.md Show resolved Hide resolved
specification/metrics-api.md Show resolved Hide resolved
specification/metrics-api.md Show resolved Hide resolved
tigrannajaryan
tigrannajaryan reviewed Jun 20, 2019
View reviewed changes
specification/metrics-api.md Outdated Show resolved Hide resolved
specification/metrics-api.md Show resolved Hide resolved
@tigrannajaryan
Copy link
Member

It would be good to add concurrency expectations for metrics to https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/concurrency.md

bogdandrutu
bogdandrutu approved these changes Jun 20, 2019
View reviewed changes
Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

I am fine with merging this and address some of the comments in a followup PR.

@mayurkale22 mayurkale22 mentioned this pull request Jun 20, 2019
Closed
@SergeyKanzhelev
Copy link
Member Author

it's active for 2 days and describe existing API. Merging.

@SergeyKanzhelev SergeyKanzhelev merged commit 3f2759f into open-telemetry:master Jun 20, 2019
@songy23 songy23 mentioned this pull request Jun 20, 2019
Closed
SergeyKanzhelev added a commit to SergeyKanzhelev/opentelemetry-specification that referenced this pull request Feb 18, 2020
@SergeyKanzhelev
Metrics API (open-telemetry#137)
17ff960 
* first draft

* without Metric

* Update specification/metrics-api.md

* metrics API

* added to the readme

* Update specification/metrics-api.md

* fixed wording for metric description

* added links to terminology

* changed link to the measurement
rockb1017 pushed a commit to rockb1017/opentelemetry-specification that referenced this pull request Nov 18, 2021
@jkwatson
Add a checklist to the GA request template. (open-telemetry#137)
5d36a0c 
* Add a checklist to the GA request template.

* Add items for configuration & semantic convention requirements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iredelmeier iredelmeier iredelmeier left review comments

@tigrannajaryan tigrannajaryan tigrannajaryan left review comments

@songy23 songy23 songy23 approved these changes

@lzchen lzchen lzchen requested changes

@reyang reyang reyang left review comments

@tedsuo tedsuo tedsuo approved these changes

@bogdandrutu bogdandrutu bogdandrutu approved these changes

@AloisReitbauer AloisReitbauer Awaiting requested review from AloisReitbauer

@c24t c24t Awaiting requested review from c24t

@carlosalberto carlosalberto Awaiting requested review from carlosalberto

@yurishkuro yurishkuro Awaiting requested review from yurishkuro

@jmacd jmacd Awaiting requested review from jmacd

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Document Java API: Metrics API
8 participants
@SergeyKanzhelev @reyang @tigrannajaryan @tedsuo @bogdandrutu @iredelmeier @songy23 @lzchen