Skip to content

Inform the user of conflicting view configuration#2608

Merged
ocelotl merged 15 commits intoopen-telemetry:mainfrom
ocelotl:issue_2556
May 6, 2022
Merged

Inform the user of conflicting view configuration#2608
ocelotl merged 15 commits intoopen-telemetry:mainfrom
ocelotl:issue_2556

Conversation

@ocelotl
Copy link
Copy Markdown
Contributor

@ocelotl ocelotl commented Apr 15, 2022

Fixes #2556

@ocelotl ocelotl requested a review from a team April 15, 2022 23:48
@ocelotl ocelotl self-assigned this Apr 15, 2022
@ocelotl ocelotl added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 15, 2022
Copy link
Copy Markdown
Member

@aabmass aabmass left a comment

Choose a reason for hiding this comment

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

Here is my take on the spec wording

Try to apply the View configuration. If applying the View results in conflicting metric identities the implementation SHOULD apply the View and emit a warning.

Conflicting identities would be as described in the link; all the identifying properties of the Metric are the same (everything but description). This is distinct from the next sentence

If it is not possible to apply the View without producing semantic errors (e.g. the View sets an asynchronous instrument to use the Explicit bucket histogram aggregation) the implementation SHOULD emit a warning and proceed as if the View did not exist.

in addition to instrument/aggregation mismatches, another other semantic error would be "the presence of multiple Metric identities for a given name with the same Resource and Scope attributes.".

It's not 100% clear to me why it's specified this way, but that's my take. Wdyt?

@ocelotl ocelotl requested a review from aabmass April 18, 2022 22:21
@ocelotl ocelotl force-pushed the issue_2556 branch 3 times, most recently from e3a8734 to c7aa3b2 Compare April 26, 2022 21:52
@ocelotl ocelotl force-pushed the issue_2556 branch 2 times, most recently from f29b9ec to 50e2de3 Compare April 27, 2022 22:24
@ocelotl ocelotl force-pushed the issue_2556 branch 2 times, most recently from 0e22309 to 6c36e35 Compare May 4, 2022 23:36
@ocelotl ocelotl requested a review from lzchen May 4, 2022 23:48
@ocelotl ocelotl requested a review from lzchen May 5, 2022 21:09
Copy link
Copy Markdown
Member

@aabmass aabmass left a comment

Choose a reason for hiding this comment

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

Approving in advance

@ocelotl ocelotl enabled auto-merge (squash) May 6, 2022 18:48
@ocelotl ocelotl merged commit 07a5b64 into open-telemetry:main May 6, 2022
@ocelotl ocelotl deleted the issue_2556 branch May 6, 2022 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip Changelog PRs that do not require a CHANGELOG.md entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Let the user know if there is a view name conflict

3 participants