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

chore: move traces builder query attributes enrichment before query prep #5917

Merged
merged 7 commits into from
Sep 13, 2024

Conversation

srikanthccv
Copy link
Member

Summary

Currently, the attribute erichment is happening at the time of query preparation. This means we should pass keys all the way down which makes the whole call require passing keys. This commit removes it from params and enriches the attribute prior like we do for logs.

@github-actions github-actions bot added the chore label Sep 10, 2024
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@srikanthccv srikanthccv marked this pull request as ready for review September 11, 2024 04:52
@makeavish makeavish linked an issue Sep 11, 2024 that may be closed by this pull request
@makeavish
Copy link
Member

@CodiumAI-Agent /review

@CodiumAI-Agent
Copy link

CodiumAI-Agent commented Sep 11, 2024

PR Reviewer Guide 🔍

(Review updated until commit ecb8c26)

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Key issues to review

Code Refactor
The removal of the keys parameter from the QueryRange method simplifies the method signature and potentially improves maintainability. However, it's crucial to ensure that all functionalities related to attribute keys are preserved or properly handled elsewhere in the codebase.

Code Refactor
The removal of the keys parameter from various methods like runBuilderQuery and runBuilderExpression simplifies the method signatures. Ensure that the functionality related to these keys is either no longer needed or is handled appropriately elsewhere.

Code Refactor
The QueryRange method's signature has been simplified by removing the keys parameter. This change should be thoroughly tested to ensure that it does not affect the functionality of querying and that any necessary attribute key handling is performed before this point.

Code Refactor
The removal of keys from methods like getColumnName and buildTracesQuery could simplify the code but requires careful consideration to ensure that all functionalities related to the keys are correctly handled or are no longer necessary.

@srikanthccv
Copy link
Member Author

@makeavish the linked issue won't be solved by this yet. The keys are still loaded every time for query range request. This refactor streamlines things so that alerts and query range perform attribute enrichment upfront.

@srikanthccv
Copy link
Member Author

The follow up built on top of this #5925

@CodiumAI-Agent
Copy link

Persistent review updated to latest commit ecb8c26

1 similar comment
@CodiumAI-Agent
Copy link

Persistent review updated to latest commit ecb8c26

@srikanthccv
Copy link
Member Author

@makeavish please review

@srikanthccv srikanthccv merged commit a5f3a18 into develop Sep 13, 2024
13 checks passed
@srikanthccv srikanthccv deleted the traces-enrich branch September 13, 2024 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants