Skip to content

Conversation

@sl0thentr0py
Copy link
Member

@sl0thentr0py sl0thentr0py commented Aug 29, 2024

This commit makes incoming distributed tracing work and finishes implementing continue_trace

  • Updates the span processor to consider remote parents as root spans
  • Adds a simpler start_span to POTelScope for now that is a simplified version of the original that just creates a POTelSpan
  • continue_trace now adds a fake remote NonRecordingSpan with the correct trace_id, parent_span_id and parent_sampled fields taken from the PropagationContext
  • fix the trace_id and span_id properties on POTelSpan

closes #3476

@codecov
Copy link

codecov bot commented Aug 29, 2024

❌ 3863 Tests Failed:

Tests completed Failed Passed Skipped
16709 3863 12846 1916
View the top 3 failed tests by shortest run time
tests.integrations.celery.test_celery test_abstract_task[<lambda>2]
Stack Traces | 0s run time
No failure message available
tests.integrations.celery.test_celery test_simple_without_performance[<lambda>2]
Stack Traces | 0s run time
No failure message available
tests.integrations.clickhouse_driver.test_clickhouse_driver test_clickhouse_dbapi_spans_with_pii
Stack Traces | 0s run time
No failure message available

To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard

@sl0thentr0py sl0thentr0py force-pushed the neel/potel/is-remote branch 3 times, most recently from 2f4e723 to d037a10 Compare August 29, 2024 15:41
@sl0thentr0py sl0thentr0py changed the title Set the correct SpanContext in continue_trace Set the correct SpanContext in continue_trace and make distributed tracing work Aug 29, 2024
@sl0thentr0py sl0thentr0py changed the title Set the correct SpanContext in continue_trace and make distributed tracing work Set the correct SpanContext in continue_trace Aug 29, 2024
@sl0thentr0py
Copy link
Member Author

working distributed trace

tested with

image
image

Copy link
Contributor

@sentrivana sentrivana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻 👍🏻 👍🏻 lgtm


def start_span(self, span=None, custom_sampling_context=None, **kwargs):
# type: (Optional[POTelSpan], Optional[SamplingContext], Any) -> POTelSpan
# TODO-neel-potel ideally want to remove the span argument, discuss with ivana
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the only use for this was using the Transaction from continue_trace, let's get rid of the arg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants