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

feat(tag_cardinality_limit transform): enable per metric limits for tag_cardinality_limit #22077

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

esensar
Copy link
Contributor

@esensar esensar commented Dec 23, 2024

Summary

This adds the ability to add per metric limits in tag_cardinality_limit transform, besides the global configuration that applies to all metrics. It supports matching metrics by name and optionally by namespace too.

Change Type

  • Bug fix
  • New feature
  • Non-functional (chore, refactoring, docs)
  • Performance

Is this a breaking change?

  • Yes
  • No

How did you test this PR?

Added new tests to transforms::tag_cardinality_limits::tests and also tested manually with the following configuration:

sources:
  static_metrics_test:
    type: static_metrics
    interval_secs: 1
    metrics:
       - name: customA
         value:
           gauge:
             value: 3.0
         kind: absolute
         tags:
           custom_tag: custom_tag_1
           another_tag: test
       - name: customA
         value:
           gauge:
             value: 4.4
         kind: incremental
         tags:
           custom_tag: custom_tag_2
           another_tag: test
       - name: customA
         value:
           gauge:
             value: 4.4
         kind: incremental
         tags:
           custom_tag: custom_tag_3
           another_tag: test2
       - name: customB
         value:
           gauge:
             value: 3.0
         kind: absolute
         tags:
           custom_tag: custom_tag_1
           another_tag: test
       - name: customB
         value:
           gauge:
             value: 4.4
         kind: incremental
         tags:
           custom_tag: custom_tag_2
           another_tag: test
       - name: customB
         value:
           gauge:
             value: 4.4
         kind: incremental
         tags:
           custom_tag: custom_tag_3
           another_tag: test2

transforms:
  tag_cardinality:
    type: "tag_cardinality_limit"
    inputs: ["static_metrics_test"]
    value_limit: 2
    mode: "exact"
    per_metric_limits:
      - name: "customA"
        namespace: "static"
        value_limit: 5
        mode: "exact"

sinks:
  console:
    inputs: ["tag_cardinality"]
    target: "stdout"
    type: "console"
    acknowledgements:
      enabled: false
    encoding:
      codec: "json"

Confirmed that behavior is different for metricA and metricB (metricA didn't get its tags removed due to higher limit).

Does this PR include user facing changes?

  • Yes. Please add a changelog fragment based on our guidelines.
  • No. A maintainer will apply the "no-changelog" label to this PR.

Checklist

  • Please read our Vector contributor resources.
  • If this PR introduces changes Vector dependencies (modifies Cargo.lock), please
    run dd-rust-license-tool write to regenerate the license inventory and commit the changes (if any). More details here.

References

…tag_cardinality_limit`

This adds the ability to add per metric limits in `tag_cardinality_limit` transform, besides the
global configuration that applies to all metrics. It supports matching metrics by name and
optionally by namespace too.

Closes: vectordotdev#15743
@esensar esensar requested a review from a team as a code owner December 23, 2024 16:30
@github-actions github-actions bot added the domain: transforms Anything related to Vector's transform components label Dec 23, 2024
@esensar
Copy link
Contributor Author

esensar commented Dec 23, 2024

I think this could also support #21112 with minor modifications, but I didn't go ahead with it right away, because I didn't want to introduce yet another level of nesting in the accepted_tags map. My idea was to reuse the key currently used for metric name for that custom key too, but that would mean changing the current metric name key to something that would hard to clash with when using custom keys. Not sure if it is a good idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: transforms Anything related to Vector's transform components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Limit tag cardinality by metric in tag_cardinality_limit transform
1 participant