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

NH-94756: dbo for mysql2 #165

Merged
merged 8 commits into from
Nov 18, 2024
Merged

NH-94756: dbo for mysql2 #165

merged 8 commits into from
Nov 18, 2024

Conversation

xuan-cao-swi
Copy link
Contributor

@xuan-cao-swi xuan-cao-swi commented Nov 1, 2024

Description

Trace injection to mysql2 instrumentation

Other changes:

  1. simplify some debug log to reduce the size (e.g. span_data -> span_data.to_span_data)
  2. simplify oboe c lib check when compiling by removing unused oboe_config_get_revision
  3. return unmodified uri when test with java-collector

Test (if applicable)

Prod

@xuan-cao-swi xuan-cao-swi marked this pull request as ready for review November 1, 2024 16:35
@xuan-cao-swi xuan-cao-swi requested a review from a team as a code owner November 1, 2024 16:35
cleverchuk
cleverchuk previously approved these changes Nov 5, 2024
Copy link
Contributor

@cheempz cheempz left a comment

Choose a reason for hiding this comment

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

Looks good @xuan-cao-swi! I made some suggestions to update the file headings. More important, is it possible to add some tests to verify the comment insertion and config option control?

Copy link
Contributor

@cheempz cheempz left a comment

Choose a reason for hiding this comment

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

Getting verrrry close @xuan-cao-swi! Some minor suggestions and one ask--can you check whether these new patch tests are actually run? Not seeing them in the Rakefile task nor the GHA worfklow output.

@xuan-cao-swi
Copy link
Contributor Author

Getting verrrry close @xuan-cao-swi! Some minor suggestions and one ask--can you check whether these new patch tests are actually run? Not seeing them in the Rakefile task nor the GHA worfklow output.

I have to execute the mysql2 test case separately in test/run_tests.sh because the OtelConfig.initialize test interferes with the mysql2 patch test case. When running rake test, since it’s a single process, the defined module is shared across the entire test suite. There is no method to unprepend a module from a class, so it’s easier to run the mysql2 patch test case separately.

@xuan-cao-swi xuan-cao-swi requested a review from cheempz November 9, 2024 01:20
Copy link
Contributor

@cheempz cheempz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @xuan-cao-swi!

@xuan-cao-swi xuan-cao-swi merged commit 44e712f into main Nov 18, 2024
14 checks passed
@xuan-cao-swi xuan-cao-swi deleted the NH-94756 branch November 21, 2024 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants