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

Remove excessive istio attributes to avoid hitting the dimensions limit #765

Merged
merged 3 commits into from
May 3, 2023

Conversation

dmitryax
Copy link
Contributor

@dmitryax dmitryax commented May 2, 2023

Add a new processor attributes/istio to drop excessive istio attributes and avoid running into the dimensions limit if scraping istio metrics is enabled.

(OTL-2117)

@dmitryax dmitryax requested review from a team as code owners May 2, 2023 21:00
- istio_.*
actions:
- action: delete
key: source_cluster
Copy link
Contributor

Choose a reason for hiding this comment

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

would it make sense to add a new value and generate each action so it's more flexible as additional attributes are (un)wanted?

Copy link
Contributor Author

@dmitryax dmitryax May 2, 2023

Choose a reason for hiding this comment

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

Do you mean having another option in values.yaml for that?

I don't think we should add more configuration options to the already bloated values.yaml for this niche use case.

Overriding the set of attributes is already possible, just a bit more verbose, like:

agent:
  config:
    processors:
      attributes/istio:
        actions:
          - action: delete
            key: another_istio_attribute
          ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, the current interface to enable istio metrics is pretty unflexible.

autodetect:
  istio: true

We can probably rethink it to be able to add this list of attributes. Maybe as a follow-up effort. Let me know if you have any ideas

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, it would be great if we made this a bit more flexible. Maybe something like this?

autodetect:
  istio:
    enabled: true
    # Explain why we remove these default set of attributes
    excludeAttributes:
    - attr1
    - attr2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is the transition from bool value to the map in a backward-compatible way. We can try something out but I would rather do it as a follow-up effort not blocking the release

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed this #769

Add a new processor attributes/istio to drop excessive istio attributes and avoid running into the dimensions limit if scraping istio metrics is enabled.
@dmitryax dmitryax force-pushed the drop-excessive-istio-attributes branch from 5746e55 to d609378 Compare May 3, 2023 01:37
….tpl

Co-authored-by: Jina Jain <jina@signalfx.com>
@dmitryax dmitryax merged commit 83b8c85 into signalfx:main May 3, 2023
@dmitryax dmitryax deleted the drop-excessive-istio-attributes branch May 4, 2023 00:04
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.

5 participants