Add metrics instrumentation sqlalchemy#1645
Add metrics instrumentation sqlalchemy#1645srikanthccv merged 27 commits intoopen-telemetry:mainfrom
Conversation
| self._register_event_listener(engine, "checkout", self._pool_checkout) | ||
|
|
||
| def _get_pool_name(self): | ||
| return self.engine.pool.logging_name or "" |
There was a problem hiding this comment.
The specification didn't write what to do in case the pool_name is not provided, so I kept it empty in this case.
There is a discussion about it in the specification, I'm waiting to see what will be decided
shalevr
force-pushed
the
Metrics-instrumentation-sqlalchemy
branch
from
51746e6 to
6e84d30
Compare
shalevr
force-pushed
the
Metrics-instrumentation-sqlalchemy
branch
from
6e84d30 to
ae56830
Compare
shalevr
force-pushed
the
Metrics-instrumentation-sqlalchemy
branch
from
e97f3c5 to
eb5e1dc
Compare
shalevr
force-pushed
the
Metrics-instrumentation-sqlalchemy
branch
from
061be49 to
b22f06a
Compare
| An instrumented engine if passed in as an argument or list of instrumented engines, None otherwise. | ||
| """ | ||
| tracer_provider = kwargs.get("tracer_provider") | ||
| tracer = get_tracer(__name__, __version__, tracer_provider) |
|
|
||
| def test_metrics_one_connection(self): | ||
| pool_name = "pool_test_name" | ||
| engine = sqlalchemy.create_engine( |
There was a problem hiding this comment.
consider moving the basic engine version to test setUp
There was a problem hiding this comment.
I thought about this, but the create_engine params are not the same for all of the tests
There was a problem hiding this comment.
I'm in favor of having the basic version as setUp and test-specific changes per test but this is not a blocker by any means
.github/workflows/test.yml
Outdated
| pull_request: | ||
| env: | ||
| CORE_REPO_SHA: 042d7a7921e25936decd95addf4fb1d30afb88e2 | ||
| CORE_REPO_SHA: f5fb6b1353929cf8039b1d38f97450866357d901 |
There was a problem hiding this comment.
Reference to this change:
open-telemetry/opentelemetry-python#3115
merge with main
CHANGELOG.md
Outdated
|
|
||
| ### Added | ||
|
|
||
| - Add metrics instrumentation for sqlalchemy |
There was a problem hiding this comment.
Should go in ## Unreleased >> ### Added section
srikanthccv
left a comment
srikanthccv
left a comment
There was a problem hiding this comment.
Did a firstpass review. Is it just the connection usage instrument that should be supported? Are there any other metrics expected?
The metrics instrumentation is implemented the same way for JS-contrib |
|
Looks like that's what defined in the spec as well so it should be fine. |
...telemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/package.py
Outdated
Show resolved
Hide resolved
...ntelemetry-instrumentation-sqlalchemy/src/opentelemetry/instrumentation/sqlalchemy/engine.py
Show resolved
Hide resolved
| pull_request: | ||
| env: | ||
| CORE_REPO_SHA: d0bb12b34b0c487198c935001636b6163485a50f | ||
| CORE_REPO_SHA: 2d1f0b9f5fce62549d1338882f37b91b95881c75 |
There was a problem hiding this comment.
|
Got some problems with the old functions. I am working on it in a separate PR |
|
4 participants
Description
Add metrics instrumentation for sqlalchemy
Fixes #1143
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.