Skip to content

[opentracer] Tracer: start_span fixes#502

Merged
Kyle-Verhoog merged 3 commits intoopentracerfrom
kyle-verhoog/opentracer-487-fix
Jun 29, 2018
Merged

[opentracer] Tracer: start_span fixes#502
Kyle-Verhoog merged 3 commits intoopentracerfrom
kyle-verhoog/opentracer-487-fix

Conversation

@Kyle-Verhoog
Copy link
Copy Markdown
Member

start_span should not activate a span. This PR fixes this and adjusts the tests to use start_active_span instead.

- remove activation logic from start_span
- refactor tests to use start_active_span instead
Comment thread ddtrace/opentracer/tracer.py Outdated
ignore_active_span=ignore_active_span
)

def _start_span(self, operation_name=None, child_of=None, references=None,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

do we need this level of indirection? It's just another call in the stack without any immediate benefit (or at least I'm missing it)

@Kyle-Verhoog Kyle-Verhoog merged commit 8d947d2 into opentracer Jun 29, 2018
@Kyle-Verhoog Kyle-Verhoog deleted the kyle-verhoog/opentracer-487-fix branch June 29, 2018 12:44
Kyle-Verhoog added a commit that referenced this pull request Sep 7, 2018
* refactor start_span documentation

* fix: start_span should not activate the created span
- remove activation logic from start_span
- refactor tests to use start_active_span instead

* match precedence to OT reference implementation
Kyle-Verhoog added a commit that referenced this pull request Sep 10, 2018
* refactor start_span documentation

* fix: start_span should not activate the created span
- remove activation logic from start_span
- refactor tests to use start_active_span instead

* match precedence to OT reference implementation
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.

2 participants