add local-telemetry stack for investigating server performance #16483
Conversation
zzstoatzz
force-pushed
the
otel-oss
branch
3 times, most recently
from
8cec80b to
d5c7583
Compare
CodSpeed Performance ReportMerging #16483 will not alter performanceComparing Summary
|
zzstoatzz
requested review from
aaazzam,
chrisguidry,
cicdw,
desertaxle and
discdiver
as code owners
| if not os.environ.get("PREFECT__ENABLE_OSS_TELEMETRY"): | ||
| import prefect.telemetry.bootstrap | ||
|
|
||
| prefect.telemetry.bootstrap.setup_telemetry() | ||
| prefect.telemetry.bootstrap.setup_telemetry() |
There was a problem hiding this comment.
Could the short circuit that you need be moved to setup_telemetry or prefect.telemetry.bootstrap instead of being in prefect.main?
There was a problem hiding this comment.
hmm i had intentionally guarded the imports as well but let me see if i can move it inside
There was a problem hiding this comment.
I was thinking that the env var you're checking for will be unset in the large majority of cases, so it'd be better to be confined to that module. What's the reason for not importing that module if this environment variable is set?
There was a problem hiding this comment.
that makes sense, i was just under the impression that the imports themselves for the existing OTEL setup would interfere with the new local telemetry. but i haven't checked yet so that might not be true - will update
related to #16299
this doesn't make any performances changes yet, instead it sets up a local telemetry stack we can use to explore OTEL data in Jaeger, which should make it easier to support / confirm suspected performance issues related to that issue