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-32409 Fix failed ASGI version lookup logs #108

Merged
merged 3 commits into from
Feb 8, 2023

Conversation

tammy-baylis-swi
Copy link
Contributor

@tammy-baylis-swi tammy-baylis-swi commented Feb 8, 2023

This fixes a framework lookup at Python.*.Version span KV calculation and removes excessive warning logs as reported by customer, which are logging once per span, 3 spans per trace created by a very simple FastAPI app (their code attached to ticket, similar in testbed PR here):

[ solarwinds_apm.exporter WARNING p#1.140549948016384] Version lookup of asgi failed, so skipping: No module named 'asgi'

This is a bit of an odd approach but all of SolarWindsSpanExporter._add_info_instrumented_framework is a little odd. The exceptions were being raised (and caught to log) because we can't import asgi to get its version. Instead, ASGI design is implemented by several frameworks listed here and added to new _ASGI_APP_IMPLEMENTATIONS and _ASGI_SERVER_IMPLEMENTATIONS. There are two new loops depending on if SpanKind.SERVER or any other kind (e.g. INTERNAL, CLIENT). Each loop goes through one implementations list and stops when it can set e.g. Python.Fastapi.Version as 0.89.1 or Python.Uvicorn.Version as 0.20.0. It won't try to set Python.Asgi.Version anymore.

Caveat: The new loops stop at the first framework they're able to import. We're making these "estimates" right now as a placeholder until there is more demand for this feature. Then we'd address the TODO and find how to be more precise than relying only on span.instrumentation_scope.name (opentelemetry.instrumentation.asgi) and SpanKind for setting Python.*.Version.

Example trace from simple ASGI app, with two Python.Fastapi.Version spans and one Python.Uvicorn.Version span: https://my.na-01.cloud.solarwinds.com/140638900734749696/traces/42280A9C6F489D8603D60B14FE80A322/54CD29BC456800EF/details

Compared trace from main, which is missing Python.*.Version KVs on all spans: https://my.na-01.cloud.solarwinds.com/140638900734749696/traces/B5A4F6F225065B391F7896B9EFD5810C/D6D2B18C2CABF10C/details. This is the actual bug. 🙂

@tammy-baylis-swi tammy-baylis-swi requested review from cheempz and a team February 8, 2023 00:11
@tammy-baylis-swi tammy-baylis-swi merged commit 68932f5 into main Feb 8, 2023
@tammy-baylis-swi tammy-baylis-swi deleted the NH-32409-fix-asgi-version-lookups-spam branch February 8, 2023 19:23
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.

2 participants