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

refactor: optimized logic for identifying if the database is Redis from the query #931

Open
wants to merge 12 commits into
base: perf/optimize-sql-instrumentation
Choose a base branch
from

Conversation

Angith
Copy link
Contributor

@Angith Angith commented Oct 8, 2024

The usage of regular expressions imposes performance overhead on SQL span creation. This PR consists of the change for a solution to that. Here a map is used to check if the span is created for the Redis database.

@Angith Angith marked this pull request as ready for review October 8, 2024 08:54
@Angith Angith requested a review from a team as a code owner October 8, 2024 08:54
Copy link
Member

@nithinputhenveettil nithinputhenveettil left a comment

Choose a reason for hiding this comment

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

Good job with the code refactoring, Angith! 🎉
I have just added a few comments.

context.go Outdated Show resolved Hide resolved
context.go Outdated Show resolved Hide resolved
@nithinputhenveettil nithinputhenveettil added the tekton_ci Add this label to start running Tekton pipelines label Oct 15, 2024
context.go Show resolved Hide resolved
Copy link
Contributor

@sanojsubran sanojsubran left a comment

Choose a reason for hiding this comment

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

@Angith Please check my review comments. We can discuss if you have any queries.

instrumentation_sql.go Outdated Show resolved Hide resolved
instrumentation_sql.go Show resolved Hide resolved
instrumentation_sql.go Outdated Show resolved Hide resolved
instrumentation_sql.go Show resolved Hide resolved
instrumentation_sql_test.go Outdated Show resolved Hide resolved
Angith and others added 4 commits October 21, 2024 11:43
Co-authored-by: sanoj subran <sanoj.subran@ibm.com>
Co-authored-by: sanoj subran <sanoj.subran@ibm.com>
Co-authored-by: sanoj subran <sanoj.subran@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tekton_ci Add this label to start running Tekton pipelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants