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

Trilogy: instrument connect and ping #704

Merged

Conversation

casperisfine
Copy link
Contributor

These can be just as slow as a query if not slower.

These can be just as slow as a query if not slower.
@casperisfine casperisfine force-pushed the trilogy-instrument-connect branch from 8e3f01c to ae76b4e Compare October 20, 2023 14:54
@@ -48,6 +48,28 @@ module Client # rubocop:disable Metrics/ModuleLength

FULL_SQL_REGEXP = Regexp.union(MYSQL_COMPONENTS.map { |component| COMPONENTS_REGEX_MAP[component] })

def initialize(options = {})
@connection_options = options # This is normally done by Trilogy#initialize
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bit of a hack, it may make sense to refactor the code upstream a tiny bit to make it less hacky here.

E.g. do the initialization proper in initialize and extract the connection in something like _connect.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fine with me. We already have initialize calling _initialize, so if we do the ivar set in Ruby instead we can patch _initialize here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So thanks to you merging trilogy-libraries/trilogy#130, in future versions of trilogy we might be able to just decorate _connect and get rid of this hack.

But overall I don't think it's a big deal, just a bit fragile if upstream refactor things out.

casperisfine pushed a commit to casperisfine/trilogy that referenced this pull request Oct 20, 2023
…nect

This is mostly to allow things like open-telemetry/opentelemetry-ruby-contrib#704
to be cleaner and more robust.

This way they can decorate `_connect` and access `@connection_options`
without having to assign it themselves.
@arielvalentin arielvalentin merged commit 6e7f8da into open-telemetry:main Oct 21, 2023
46 checks passed
Hareramrai pushed a commit to tools-aoeur/opentelemetry-ruby-contrib that referenced this pull request Nov 2, 2023
* chore: use stable toys version

* chore: use stable version of toys release code

* release: Release 2 gems (open-telemetry#675)

* fix: Fix "uses cases" typo in `CONTRIBUTING.md` (open-telemetry#679)

* chore(deps-dev): update webmock requirement from ~> 3.18.1 to ~> 3.19.1 in /resources/azure (open-telemetry#653)

* chore(deps-dev): update webmock requirement from ~> 3.18.1 to ~> 3.19.1 in /resources/google_cloud_platform (open-telemetry#652)

* fix: Remove dependence on activesupport (open-telemetry#687)

Instrumentations must not use transitive dependencies used by the library.

In this case, relying on the gruf library to load specific ActiveSupport extensions leaves the instrumentation vulnerable to bugs.

For this reason I have changed the code to use Enumerable methods instead of ActiveSupport extensions.

See open-telemetry#686

* fix!: Drop DelayedJob ActiveRecord in Tests (open-telemetry#685)

* fix: Add Rails 7.1 compatability (open-telemetry#684)

* chore: Add tests for Rails 7.1

* fix: Rails 7.1 incompatabilities

* feat!: obfuscation for mysql2, dalli and postgresql as default option for db_statement (open-telemetry#682)

* feat!: fuscation for mysql2, dalli and pg

* feat!: update readme

* feat!: set db.statement option to obfuscate by default for mysql2, pg and dalli

Co-authored-by: Ariel Valentin <arielvalentin@users.noreply.github.com>

* ci: Update test reset code for GraphQL-Ruby 2.1.3 (open-telemetry#691)

Fixes open-telemetry#689

* fix: Omit `nil` `net.peer.name` attributes (open-telemetry#693)

* ci: upgrade to latest stable version of toys (open-telemetry#694)

Fixes errors with incompatible versions defined in the configs:

https://github.com/open-telemetry/opentelemetry-ruby-contrib/actions/runs/6534421626/job/17741560442#step:5:10

* release: Release 12 gems (open-telemetry#695)

* release: Release 12 gems

* opentelemetry-instrumentation-gruf 0.1.1 (was 0.1.0)
* opentelemetry-instrumentation-active_support 0.4.3 (was 0.4.2)
* opentelemetry-instrumentation-action_view 0.6.1 (was 0.6.0)
* opentelemetry-instrumentation-action_pack 0.7.1 (was 0.7.0)
* opentelemetry-instrumentation-active_job 0.6.1 (was 0.6.0)
* opentelemetry-instrumentation-active_record 0.6.3 (was 0.6.2)
* opentelemetry-instrumentation-dalli 0.25.0 (was 0.24.2)
* opentelemetry-instrumentation-delayed_job 0.22.0 (was 0.21.0)
* opentelemetry-instrumentation-faraday 0.23.3 (was 0.23.2)
* opentelemetry-instrumentation-mysql2 0.25.0 (was 0.24.3)
* opentelemetry-instrumentation-pg 0.26.0 (was 0.25.3)
* opentelemetry-instrumentation-rails 0.28.1 (was 0.28.0)

* ci: Trigger builds

* fix: Apply suggestions from code review

Co-authored-by: Josef Šimánek <josef.simanek@gmail.com>

* fix: bump gem versions

* fix: bump gems

---------

Co-authored-by: Ariel Valentin <ariel@arielvalentin.com>
Co-authored-by: Ariel Valentin <arielvalentin@github.com>
Co-authored-by: Ariel Valentin <arielvalentin@users.noreply.github.com>
Co-authored-by: Josef Šimánek <josef.simanek@gmail.com>

* chore: Update CODEOWNERS (open-telemetry#692)

* chore: Update CODEOWNERS

Add @simi and @kaylareopelle and approvers to the list

* release: Release opentelemetry-instrumentation-all 0.51.0 (was 0.50.1) (open-telemetry#699)

* release: Release opentelemetry-instrumentation-all 0.51.0 (was 0.50.1)

* docs: Update all gem Changelog

---------

Co-authored-by: Ariel Valentin <ariel@arielvalentin.com>
Co-authored-by: Ariel Valentin <arielvalentin@users.noreply.github.com>

* feat(trilogy): instrument connect and ping (open-telemetry#704)

These can be just as slow as a query if not slower.

Co-authored-by: Jean Boussier <jean.boussier@gmail.com>

* fix: Remove dependency on ActiveSupport core extensions from Grape instrumentation (open-telemetry#706)

* release: Release opentelemetry-instrumentation-trilogy 0.57.0 (was 0.56.3) (open-telemetry#708)

* release: Release opentelemetry-instrumentation-trilogy 0.57.0 (was 0.56.3)

* Empty commit

* Release instrumentation-all as well.

---------

Co-authored-by: Ariel Valentin <ariel@arielvalentin.com>
Co-authored-by: Francis Bogsanyi <francis.bogsanyi@shopify.com>

* chore(deps): update rubocop requirement from ~> 1.56.2 to ~> 1.57.1 (open-telemetry#702)

Updates the requirements on [rubocop](https://github.com/rubocop/rubocop) to permit the latest version.
- [Release notes](https://github.com/rubocop/rubocop/releases)
- [Changelog](https://github.com/rubocop/rubocop/blob/master/CHANGELOG.md)
- [Commits](rubocop/rubocop@v1.56.2...v1.57.1)

---
updated-dependencies:
- dependency-name: rubocop
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): update rubocop requirement from ~> 1.56.1 to ~> 1.57.1 in /instrumentation/base (open-telemetry#701)

chore(deps-dev): update rubocop requirement from ~> 1.56.1 to ~> 1.57.1

Updates the requirements on [rubocop](https://github.com/rubocop/rubocop) to permit the latest version.
- [Release notes](https://github.com/rubocop/rubocop/releases)
- [Changelog](https://github.com/rubocop/rubocop/blob/master/CHANGELOG.md)
- [Commits](rubocop/rubocop@v1.56.1...v1.57.1)

---
updated-dependencies:
- dependency-name: rubocop
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): update rubocop requirement from ~> 1.56.1 to ~> 1.57.1 in /resource_detectors (open-telemetry#700)

chore(deps-dev): update rubocop requirement from ~> 1.56.1 to ~> 1.57.1

Updates the requirements on [rubocop](https://github.com/rubocop/rubocop) to permit the latest version.
- [Release notes](https://github.com/rubocop/rubocop/releases)
- [Changelog](https://github.com/rubocop/rubocop/blob/master/CHANGELOG.md)
- [Commits](rubocop/rubocop@v1.56.1...v1.57.1)

---
updated-dependencies:
- dependency-name: rubocop
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): update rubocop requirement from ~> 1.56.1 to ~> 1.57.1 in /propagator/ottrace (open-telemetry#698)

chore(deps-dev): update rubocop requirement from ~> 1.56.1 to ~> 1.57.1

Updates the requirements on [rubocop](https://github.com/rubocop/rubocop) to permit the latest version.
- [Release notes](https://github.com/rubocop/rubocop/releases)
- [Changelog](https://github.com/rubocop/rubocop/blob/master/CHANGELOG.md)
- [Commits](rubocop/rubocop@v1.56.1...v1.57.1)

---
updated-dependencies:
- dependency-name: rubocop
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): update rubocop requirement from ~> 1.56.1 to ~> 1.57.1 in /propagator/xray (open-telemetry#697)

chore(deps-dev): update rubocop requirement from ~> 1.56.1 to ~> 1.57.1

Updates the requirements on [rubocop](https://github.com/rubocop/rubocop) to permit the latest version.
- [Release notes](https://github.com/rubocop/rubocop/releases)
- [Changelog](https://github.com/rubocop/rubocop/blob/master/CHANGELOG.md)
- [Commits](rubocop/rubocop@v1.56.1...v1.57.1)

---
updated-dependencies:
- dependency-name: rubocop
  dependency-type: direct:development
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Remove opentelemetry-resource_detectors all-in-one gem (open-telemetry#659)

* fix: remove call to ActiveSupport::Notifications.notifier#synchronize deprecated in Rails 7.2 (open-telemetry#707)

* wrap call to depcrecated private API behind version conditional

* convert Rails version string to Gem::Version

* fix module access?

* check for synchronize method instead of Rails version

* add comment

---------

Co-authored-by: Ariel Valentin <arielvalentin@users.noreply.github.com>

* chore: Fix linter issue

* release: Release 2 gems (open-telemetry#710)

* release: Release 2 gems

* opentelemetry-instrumentation-grape 0.1.5 (was 0.1.4)
* opentelemetry-instrumentation-active_support 0.4.4 (was 0.4.3)

* ci: Force

---------

Co-authored-by: Ariel Valentin <ariel@arielvalentin.com>
Co-authored-by: Ariel Valentin <arielvalentin@github.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Ariel Valentin <arielvalentin@github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Sam Bostock <sambostock@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ariel Valentin <arielvalentin@users.noreply.github.com>
Co-authored-by: Xuan <112967240+xuan-cao-swi@users.noreply.github.com>
Co-authored-by: Robert Mosolgo <rdmosolgo@gmail.com>
Co-authored-by: Yohei Kitamura <3087402+yoheyk@users.noreply.github.com>
Co-authored-by: Ariel Valentin <ariel@arielvalentin.com>
Co-authored-by: Josef Šimánek <josef.simanek@gmail.com>
Co-authored-by: Jean byroot Boussier <jean.boussier+github@shopify.com>
Co-authored-by: Jean Boussier <jean.boussier@gmail.com>
Co-authored-by: Muriel <m.picone.farias@catawiki.nl>
Co-authored-by: Francis Bogsanyi <francis.bogsanyi@shopify.com>
Co-authored-by: Sander Jans <scbjans@gmail.com>
Co-authored-by: Katherine Oelsner <49968061+octokatherine@users.noreply.github.com>
@github-actions github-actions bot mentioned this pull request Jan 5, 2024
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.

6 participants