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

Conditional attributes processor #1823

Merged

Conversation

tamirdavid1
Copy link
Collaborator

No description provided.

@RonFed
Copy link
Contributor

RonFed commented Nov 23, 2024

General questions:

  1. Is the main goal here is to improve CPU consumption from the old approach?
  2. Do we know how much CPU was consumed by the old processor (in terms of percentages)?
  3. How is this change improving the CPU consumption relative to the current approach? Did we measure the difference?
  4. Do we need to delete the old implementation? I didn't see that in the PR but maybe I missed that.

@tamirdavid1
Copy link
Collaborator Author

tamirdavid1 commented Nov 24, 2024

General questions:

  1. Is the main goal here is to improve CPU consumption from the old approach?
  2. Do we know how much CPU was consumed by the old processor (in terms of percentages)?
  3. How is this change improving the CPU consumption relative to the current approach? Did we measure the difference?
  4. Do we need to delete the old implementation? I didn't see that in the PR but maybe I missed that.

Before starting responding, this is the first phase of this change the next phase will include creating an action that will create this processor with needed configuration. after validation I will delete the old approach.

Link to the old approach of setting category attributes:
https://github.com/odigos-io/odigos/blob/main/cli/cmd/resources/profiles/category-attributes.yaml

Hey @RonFed ,

  1. Yes the main goal is to improve the CPU compared to the old approach (transform processor)
  2. We dont know how much CPU was consumed but it was high cpu consumer, imagine that for each span we went over the huge list of the category attribute that include also regex
  3. In the suggested approach for each condition we can ask, what are the new attributes values based on another attribute with value X.
    It uses map so dont need to iterate just ask values[odigos.io/ebpf/instrumentation-fastify]
    image

    For your question we didnt measure the improvement yet, will do so in the next phase of this change.
  4. after i'll make sure that it works i will remove the old approach

@tamirdavid1 tamirdavid1 force-pushed the conditional-attributes-processor branch from 0dc8b18 to 0b2a301 Compare November 24, 2024 15:07
@tamirdavid1 tamirdavid1 force-pushed the conditional-attributes-processor branch from d274c10 to bf2122b Compare November 24, 2024 15:16
@tamirdavid1 tamirdavid1 merged commit ef8c7f5 into odigos-io:main Nov 25, 2024
29 checks passed
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.

3 participants