Skip to content

[receiver/snmpreceiver] fix scalar RA # check #26363

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

Conversation

kuiperda
Copy link
Contributor

@kuiperda kuiperda commented Aug 31, 2023

Description:
Fixed a bug introduced by #25174

There was check that was supposed to ensure that all resource attributes added to a metric were scalar, but it was really checking if the number of resource attributes on a metric was the same as the total number of possible scalar resource attributes defined in the config. This check has been updated to explicitly verify that all of the resource attributes on the metric are scalar.

Bug examples:

  • A column metric indexed by a column attribute has two scalar resource attributes with different names but the same oid value. They are the only two scalar RAs defined in the config. Since the oids are added to a map whose length is checked, they are seen as one value and the comparison fails so many resources are created instead of one.
  • A column metric indexed by a column attribute has only one scalar resource attribute when two possible attributes are defined in the config. The check fails and many resources are created instead of one.

Testing:

  • Test that a metric with one scalar resource attribute when two are available still creates only one resource
  • First bug does not feel necessary to unit test, it has been tested locally and is fixed.

Documentation:

  • N/A

Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

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

LGTM. Just one nit.

@djaglowski djaglowski merged commit 06c7b3d into open-telemetry:main Sep 5, 2023
@djaglowski djaglowski deleted the fix/make-snmpreceiver-ra-check-explicit branch September 5, 2023 16:42
@github-actions github-actions bot added this to the next release milestone Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants