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

Add IgnoreResourceAttributeValue CompareOption #10828

Merged

Conversation

antonblock
Copy link
Contributor

Description: Adds IgnoreResourceAttributeValue CompareOption, copying pattern of IgnoreMetricAttributeValue. This new option removes resource attributes from expected and actual metrics when comparing metrics. This is desirable for integration tests in cases where a resource attribute is unpredictable, for example random UUID or MAC address.

Link to tracking Issue: #10129

Testing: Additional test cases were added to TestCompareMetrics using the new option. The cases test masking the sole resource attribute from a resource, and removing masking one attribute but preserving other predictable attributes.

Documentation: The exported IgnoreResourceAttributeValue method is documented with a description of what it does

@antonblock antonblock marked this pull request as ready for review June 7, 2022 16:55
@antonblock antonblock requested a review from a team June 7, 2022 16:55
@antonblock antonblock requested a review from djaglowski as a code owner June 7, 2022 16:55
Copy IgnoreMetricAttributeValue pattern to remove specified resource
attribute for comparison. Add unit tests.
@antonblock antonblock force-pushed the ignore-resource-attribute-value branch from 1a49e61 to 1e2d848 Compare June 8, 2022 12:49
@djaglowski
Copy link
Member

I added a test to see how easy it is to induce a false failure, and unfortunately it appears to be problematic. That said, I think there are cases where we can still benefit from this functionality as is, such as #10165.

It seems to me that to handle unordered slices of resources/scopes/datapoints, we should add sortation functions to this package, which could be called before comparisons are made. The sortation likely needs to be sensitive to the same CompareOptions so that expected variables (unpredictable values, times, etc) do not influence the sort. I'll open another issue for this.

@djaglowski djaglowski merged commit a7ab40a into open-telemetry:main Jun 8, 2022
kentquirk pushed a commit to McSick/opentelemetry-collector-contrib that referenced this pull request Jun 14, 2022
* Add IgnoreResourceAttributeValue CompareOption

Copy IgnoreMetricAttributeValue pattern to remove specified resource
attribute for comparison. Add unit tests.

* Update CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants