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

Clarify metric SDK views section #3524

Merged
merged 18 commits into from
Jun 29, 2023

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented May 31, 2023

  • Break the metric SDK views section into sub-sections. This provided structure helps define normative requirements hierarchically and clearly (required inputs are separated from their definitions).
    • Split the list of view inputs into their own sections.
    • Split the suggested implementation of View resolution into its own section.
    • Split the Python examples into their own section.
  • Clarify the normative requirements and optionality to the views input. Currently the input parameters are (mostly) accompanied by terms like (required) or (optional). These terms have not normative value in the specification and are not clear about who their subject is (i.e. SDK author or SDK user).
    • Clarify the instrument selection criteria are all required to be implemented but optional for a user to provide.
    • Clarify the stream configuration parameters are all required to be implemented but optional for a user to provide.
  • Move the view name to be the name parameter of a stream configuration.

@MrAlias MrAlias added the editorial Editorial changes only (typos, changelog, ...). No content-related changes of any kind. label May 31, 2023
@MrAlias MrAlias requested review from a team May 31, 2023 21:11
specification/metrics/sdk.md Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
Copy link
Member

@pellared pellared left a comment

Choose a reason for hiding this comment

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

@MrAlias Feel free to resolve my comments.

specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
MrAlias and others added 3 commits June 5, 2023 07:46
Co-authored-by: Georg Pirklbauer <georg.pirklbauer@dynatrace.com>
Co-authored-by: Robert Pająk <pellared@hotmail.com>
@github-actions
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jun 17, 2023
Copy link
Contributor

@jmacd jmacd left a comment

Choose a reason for hiding this comment

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

I see a the concerns about optionality to the user and optionality to the SDK and would be glad to see those improved in a future PR. I think this change is a big improvement either way.

specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Outdated Show resolved Hide resolved
specification/metrics/sdk.md Show resolved Hide resolved
@reyang reyang merged commit 3311348 into open-telemetry:main Jun 29, 2023
@MrAlias MrAlias deleted the metric-views-refactor branch June 29, 2023 20:18
@MrAlias MrAlias added this to the Metrics Future Release milestone Aug 3, 2023
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial Editorial changes only (typos, changelog, ...). No content-related changes of any kind. Stale
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

9 participants