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

[processor/transform] Add ability to limit attributes #9552

Merged
merged 22 commits into from
May 6, 2022

Conversation

TylerHelmuth
Copy link
Member

Description:
This PR adds to the transform processor the ability to limit the number of items in a map. The intended goal being the ability to limit the number of attributes and resource.attributes.

Link to tracking Issue:
#9366
This is one of two PR I have submitted for this issue.

Testing:
Added unit tests and tested locally running the collector and tracegen

Documentation:
Updated README

@TylerHelmuth
Copy link
Member Author

This should be merged after #9546

@TylerHelmuth
Copy link
Member Author

@open-telemetry/collector-contrib-approvers @anuraaga this is now ready to be reviewed.

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.

This can be a separate issue, but I wonder if the fully random behavior will limit the usefulness of this function. Let's say I have a small number of very important attributes but would need to limit the more arbitrary ones. It seems I have no control to preserve those important keys.

Perhaps something like limit(target, limit, priority_keys...) would be a natural extension for this case.

processor/transformprocessor/README.md Outdated Show resolved Hide resolved
@TylerHelmuth
Copy link
Member Author

I like that idea. Any idea how instrumentations are dealing with the randomness of the drops?

@TylerHelmuth
Copy link
Member Author

I like that idea. Any idea how instrumentations are dealing with the randomness of the drops?

Looking through some instrumentations, it looks like the limits are applied as attributes are being added. If you add in a big bunch, Java will randomly drop, but if you add one at a time, the previously added attributes take precedence.

For the collector, since most attributes are already applied, I think the addition of a "never drop these attributes" feature is a good add. I will open a new issue for this.

@TylerHelmuth
Copy link
Member Author

I will open a new issue for this.

Issue opened: #9734

@TylerHelmuth
Copy link
Member Author

@open-telemetry/collector-contrib-approvers please review.

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

@djaglowski djaglowski merged commit f21f89e into open-telemetry:main May 6, 2022
@TylerHelmuth TylerHelmuth deleted the issue-9366-limit branch May 6, 2022 18:38
djaglowski pushed a commit to djaglowski/opentelemetry-collector-contrib that referenced this pull request May 10, 2022
…#9552)

* Add truncation function

* Add TODO

* Updated changelog

* Updated changelog

* Update name to truncateAll

* Fixed README

* add limit function

* Updated changelog

* Updated go.sum

* Add check to avoid loop

* added checked for negative limit

* fix README.md

* Fix lint issue

* Add link to issue for writing logs

Co-authored-by: Bogdan Drutu <bogdandrutu@gmail.com>
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.

4 participants