Fix unexpected 'No active span' IllegalStateError #311
Conversation
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
skywalking/log/sw_logging.py
Outdated
| @@ -83,7 +83,7 @@ def build_log_tags() -> LogTags: | |||
| # exceptions before any span is even created, just ignore these fields and | |||
| # avoid appending 'no active span' traceback that could be confusing. | |||
| # Or simply the log is generated outside any span context. | |||
| active_span_id = context.active_span.sid | |||
| active_span_id = context.peek(raise_if_none=True).sid | |||
There was a problem hiding this comment.
reportOptionalMemberAccess: "sid" is not a known member of "None"
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
skywalking/plugins/sw_loguru.py
Outdated
| @@ -79,7 +79,7 @@ def _sw_sink(message): | |||
| # exceptions before any span is even created, just ignore these fields and | |||
| # avoid appending 'no active span' traceback that could be confusing. | |||
| # Or simply the log is generated outside any span context. | |||
| active_span_id = context.active_span.sid | |||
| active_span_id = context.peek(raise_if_none=True).sid | |||
There was a problem hiding this comment.
reportOptionalMemberAccess: "sid" is not a known member of "None"
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
skywalking/log/sw_logging.py
Outdated
| active_span = context.active_span | ||
| if active_span is not None: | ||
| active_span_id = active_span.sid | ||
| primary_endpoint_name = context.primary_endpoint.get_name() |
There was a problem hiding this comment.
reportOptionalMemberAccess: "get_name" is not a known member of "None"
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
skywalking/plugins/sw_loguru.py
Outdated
| active_span = context.active_span | ||
| if active_span is not None: | ||
| active_span_id = active_span.sid | ||
| primary_endpoint_name = context.primary_endpoint.get_name() |
There was a problem hiding this comment.
reportOptionalMemberAccess: "get_name" is not a known member of "None"
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
Fix unexpected 'No active span' IllegalStateError (apache#311)
|
Good catch! Thanks for the fix. |
|
CI fails, please fix them first. |
|
Probably needs a full rerun， the old image got deleted last time. |
|
Kafka cases are flaky, not related to this PR or agent functionalities, will fix. |
The skywalking.trace.context.SpanContext.active_span() method has been refactored in v1.0.0. Its equivalent method has been changed to peek() . When peek() returns None, the active_span() method throws IllegalStateError exception directly.
This change is not backward compatible, SOME plugins that rely on the returned None value of active_span() WILL be broken ( sw_flask, sw_sanic, sw_tornado, sw_django, etc.).
For example, when running Flask by gunicorn which not monkey-patched as well as for sw_http_server plugin, calling get_context().active_span in _sw_handle_exception() will raise an IllegalStateError exception directly and break the original logic of the container's handle_exception() .
In v0.8.0, no exception was thrown, so the caller could check if entry_span is not None and perform further processing.
Therefore, I suggest retaining the behavioral pattern of the active_span() method in v0.8.0 , and providing parameters in the added peek() method to support scenarios where exceptions need to be thrown (such as in sw_logging / sw_loguru).
CHANGELOG.md.