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

An easier way to partial match metric attributes is needed for the metricdatatest assert functions #3435

Closed
MrAlias opened this issue Nov 1, 2022 · 5 comments
Labels
area:metrics Part of OpenTelemetry Metrics enhancement New feature or request pkg:SDK Related to an SDK package pkg:testing Related to testing or a testing package

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Nov 1, 2022

Problem Statement

I'm always frustrated when I need to assert an incomplete set of attributes exist for a metric using the metricdatatest assert functions.

For example, I know all of my sums should contain the attribute.String("statement", "INSERT") attribute, but they will also contain an attribute describing the host. The host changes every test, therefore, I only want to assert the attribute set contains the known attribute and not fail because it also contains extra attributes.

Proposed Solution

Add an option to support this

Alternatives

Define a new type in the attribute package that acts as a "minimal set" and when compared to a Set it would do the desired comparison.

@MrAlias MrAlias added enhancement New feature or request pkg:testing Related to testing or a testing package pkg:SDK Related to an SDK package area:metrics Part of OpenTelemetry Metrics labels Nov 1, 2022
@paivagustavo
Copy link
Member

This is probably about the default resources (sorry if it is not), would there be a way to create a mock for the default resources during tests, making them deterministic or at least using an empty resource for those cases?

I feel like having exact matches for tests is better and lead to less buggy code than checking for a subset of things.

@MrAlias
Copy link
Contributor Author

MrAlias commented Nov 3, 2022

This is probably about the default resources (sorry if it is not), would there be a way to create a mock for the default resources during tests, making them deterministic or at least using an empty resource for those cases?

I feel like having exact matches for tests is better and lead to less buggy code than checking for a subset of things.

It was for metric instruments: https://github.com/open-telemetry/opentelemetry-go-contrib/pull/2962/files#diff-4952dc7ae3d29c757b4554040c9b95d338aeb80ce6f88691cf17f845280f78e6R378-R392

Though I do wonder if we should provide something that says "check for the default resource".

@MrAlias
Copy link
Contributor Author

MrAlias commented Dec 19, 2022

#3487 adds a function to assert a type has a set of attributes, but it does not resolve this issue. It is still not possible to compare a minimal set of attributes are contained by a type.

The function added in #3487 may need to be updated to accept a configuration to signal this type of comparison, or another function will need to be added. We need to have a plan prior to GA as this will be an API breaking change to add a config.

@MadVikingGod
Copy link
Contributor

#3487 does allow for a partial match. It specifically checks if attrs is a subset of every attribute.Set in the collection or datapoint.

Here is an example: https://go.dev/play/p/2gAJjYE_SC0
If any of the datapoints did not have "statement": "INSERT" in their attributes it would fail.

@MrAlias
Copy link
Contributor Author

MrAlias commented Jan 3, 2023

#3487 does allow for a partial match. It specifically checks if attrs is a subset of every attribute.Set in the collection or datapoint.

Here is an example: https://go.dev/play/p/2gAJjYE_SC0 If any of the datapoints did not have "statement": "INSERT" in their attributes it would fail.

Hmm, yeah, that looks correct. I'm not sure why I thought otherwise 😕.

Closing then as this looks resolved by #3487

@MrAlias MrAlias closed this as completed Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics enhancement New feature or request pkg:SDK Related to an SDK package pkg:testing Related to testing or a testing package
Projects
No open projects
Status: Done
Development

No branches or pull requests

3 participants