Fix requests and urllib instrumentations span name callback parameters#259
Fix requests and urllib instrumentations span name callback parameters#259lzchen merged 4 commits intoopen-telemetry:masterfrom
Conversation
| span_name = "" | ||
| if name_callback is not None: | ||
| span_name = name_callback() | ||
| span_name = name_callback(method, url) |
There was a problem hiding this comment.
Just a thought that although this is sufficient for the vast majority of cases, I have a feeling people may want to customize the span name further later on.
For example, one cannot extracted values from the header and add them to the span name.
I don't have a great solution (besides passing in all args via **kwargs, etc), but this may come up as an issue.
There was a problem hiding this comment.
Yeah I thought of that too. For now, I think this is sufficient and we can change the parameters if the need arises.
...on/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py
Show resolved
Hide resolved
|
QQ: why does this have the "Skip Changelog" tag? this is a new feature being added to the requests / urllib instrumentations. or at least a bugfix. |
| - `opentelemetry-instrumentation-flask` Do not emit a warning message for request contexts created with `app.test_request_context` | ||
| ([#253](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/253)) | ||
| - `opentelemetry-instrumentation-requests`, `opentelemetry-instrumentation-urllib` Fix span name callback parameters | ||
| - ([#259](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/259)) |
There was a problem hiding this comment.
Note that there's an extra dash on line 43 here.
There was a problem hiding this comment.
Probably will be fixed in a new pr
Addresses this comment