DRAFT: make cleaner integrations to prepare for auto integration project #15589
Conversation
|
|
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 248 ± 2 ms. The average import time from base is: 253 ± 4 ms. The import time difference between this PR and base is: -4.6 ± 0.1 ms. Import time breakdownThe following import paths have shrunk:
|
|
|
||
| Usage: | ||
| # During ddtrace initialization | ||
| from ddtrace._trace.tracing_plugins import initialize |
There was a problem hiding this comment.
Are the plugins going to be tracer specific? Would it make sense to put these modules in ddtrace/internal/plugins/..... I can see other products (asm, llmobs, serverless, ...) extending or using these plugins. Making them tracing specific puts us in a corner.
| _initialized = True | ||
|
|
||
| # Import contrib plugins | ||
| from ddtrace._trace.tracing_plugins.contrib import asyncpg |
There was a problem hiding this comment.
We should avoid inlining imports at all costs. This is a code smell and a sign of circular imports. If importing modules have unintended side effects we should remove them (ex: importing from ddtrace.contrib should not register trace handler listeners, we should get rid of this import side effect)
| """ | ||
|
|
||
| kind = SpanKind.CLIENT | ||
| type = "web" |
There was a problem hiding this comment.
We can use SpanType here.
| (receiving data FROM a message broker/queue). | ||
| """ | ||
|
|
||
| from typing import TYPE_CHECKING |
There was a problem hiding this comment.
We shouldn't use TYPE_CHECKING constant any more. This is used to support python2 style typing (types in comments). We should enforce python3 typing and if we do we don;t need to hid imports.
Using this is actually a code smell because it can hide circular imports
| return | ||
|
|
||
| # Mark as measured | ||
| span.set_metric(_SPAN_MEASURED_KEY, 1) |
There was a problem hiding this comment.
This is a very important tag for trace metrics. Now that we are getting rid of integration specific service names we will ONLY compute trace metrics for spans with this tag OR tags that have a span.kind of producer, server, client, and producer.
To be safe we should move this to be a core component for all plugins. This should be set on the first span for each integration
|
|
||
| def on_finish( | ||
| self, | ||
| ctx: "ExecutionContext", |
There was a problem hiding this comment.
Are we avoiding circular imports here? Can we avoid this?
| from ddtrace.constants import SPAN_KIND | ||
| from ddtrace.internal.constants import COMPONENT | ||
|
|
||
| pin = ctx.get_item("pin") |
There was a problem hiding this comment.
get rid of pin. do not use it anywhere
Description
Testing
Risks
Additional Notes