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

Refine description of Instrumentation Scope for Logs #3855

Merged
merged 28 commits into from
Mar 15, 2024
Merged

Refine description of Instrumentation Scope for Logs #3855

merged 28 commits into from
Mar 15, 2024

Conversation

pellared
Copy link
Member

@pellared pellared commented Jan 31, 2024 •
edited
Loading

Fixes #3841

Changes

  • Update "Instrumentation Scope" term in glossary.
  • Update "Attribute Collections" (in common definitions) to include instrumentation scope attributes.
  • Change the instrumentation scope type definition from (Name,Version) tuple of strings to Instrumentation Scope definition (as the current definition misses Scheme URL and Attributes).
  • Refer to InstrumentationScope Data Model field in Bridge API to make the spec easier to follow.
  • Move the part relevant to Bridge API usage from the Data Model.
  • Remove "Version is optional. Name SHOULD be specified if version is specified, otherwise Name is optional." which should act as recommendation in the API. But the API already contains information how the parameters should be used.

@pellared pellared marked this pull request as ready for review January 31, 2024 12:25
@pellared pellared requested review from a team January 31, 2024 12:25
@pellared pellared requested a review from a team January 31, 2024 13:25
@pellared pellared requested a review from arminru January 31, 2024 16:05
@pellared pellared mentioned this pull request Jan 31, 2024
Open
cijothomas
cijothomas approved these changes Feb 1, 2024
View reviewed changes
Copy link
Member

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

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

LGTM

@pellared @reyang
Update specification/common/README.md
9d26df5 
Co-authored-by: Reiley Yang <reyang@microsoft.com>
@pellared pellared requested a review from reyang February 1, 2024 19:35
@pellared pellared changed the title Refine description of InstrumentationScope in Logs Refine description of InstrumentationScope for Logs Feb 20, 2024
@pellared pellared changed the title Refine description of InstrumentationScope for Logs Refine description of Instrumentation Scope for Logs Feb 20, 2024
tigrannajaryan
tigrannajaryan requested changes Feb 27, 2024
View reviewed changes
specification/logs/data-model.md Outdated Show resolved Hide resolved
@pellared
Copy link
Member Author

@tigrannajaryan PTAL. I addressed your comment.

tigrannajaryan
tigrannajaryan approved these changes Mar 14, 2024
View reviewed changes
Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

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

LGTM

@jack-berg jack-berg merged commit 6787e14 into open-telemetry:main Mar 15, 2024
7 checks passed
@pellared pellared mentioned this pull request Jul 22, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chalin chalin chalin left review comments

@tigrannajaryan tigrannajaryan tigrannajaryan approved these changes

@cijothomas cijothomas cijothomas approved these changes

@MrAlias MrAlias MrAlias approved these changes

@jack-berg jack-berg jack-berg approved these changes

@arminru arminru Awaiting requested review from arminru

@reyang reyang Awaiting requested review from reyang

@carlosalberto carlosalberto Awaiting requested review from carlosalberto

Assignees

@carlosalberto carlosalberto

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Clarify attributes parameter type of Get a Logger operation
9 participants
@pellared @carlosalberto @chalin @tigrannajaryan @cijothomas @MrAlias @arminru @reyang @jack-berg