Skip to content

[OpenTracing] Tracer start_span and start_active_span implementation#490

Merged
delner merged 3 commits into
feature/opentracingfrom
feature/opentracing_tracer
Jul 17, 2018
Merged

[OpenTracing] Tracer start_span and start_active_span implementation#490
delner merged 3 commits into
feature/opentracingfrom
feature/opentracing_tracer

Conversation

@delner

@delner delner commented Jul 16, 2018

Copy link
Copy Markdown
Contributor

This pull request uses the other components from Datadog::OpenTrace to implement the start_span and start_active_span functions.

It does not implement the inject or extract functions, which will come in a separate pull request.

@delner delner added core Involves Datadog core libraries feature Involves a product feature labels Jul 16, 2018
@delner delner self-assigned this Jul 16, 2018

@Kyle-Verhoog Kyle-Verhoog left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good

ignore_active_scope: ignore_active_scope
)

scope_manager.activate(span, finish_on_close: finish_on_close).tap do |scope|

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

# References#CHILD_OF reference to the ScopeManager#active.
# @return [Span] the newly-started Span instance, which has not been
# automatically registered via the ScopeManager
def start_span(operation_name,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍 👍

@pawelchcki pawelchcki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Implementation and tests both look good to me 👍

@delner

delner commented Jul 17, 2018

Copy link
Copy Markdown
Contributor Author

Thanks for the reviews guys. Think I'm going to add a few more tests to verify basic features (like tags, etc.)

@delner
delner force-pushed the feature/opentracing branch from 277294b to 45ab187 Compare July 17, 2018 19:57
@delner
delner force-pushed the feature/opentracing_tracer branch from c7f386c to b1028be Compare July 17, 2018 19:57
@delner
delner force-pushed the feature/opentracing_tracer branch from b1028be to ca4a379 Compare July 17, 2018 20:47
@delner

delner commented Jul 17, 2018

Copy link
Copy Markdown
Contributor Author

Found a small bug where the ThreadLocalScopeManager wasn't passing finish_on_close to its ThreadLocalScope on #activate.

@delner
delner merged this pull request into feature/opentracing Jul 17, 2018
delner added a commit that referenced this pull request Jul 19, 2018
…490)

 * Fixed: ThreadLocalScopeManager not passing finish_on_close to scope.

 * Added: Implementation for OpenTracer::Tracer

 * Added: OpenTracing::Tracer integration specs.
delner added a commit that referenced this pull request Aug 9, 2018
…490)

 * Fixed: ThreadLocalScopeManager not passing finish_on_close to scope.

 * Added: Implementation for OpenTracer::Tracer

 * Added: OpenTracing::Tracer integration specs.
@delner
delner deleted the feature/opentracing_tracer branch August 14, 2018 17:42
delner added a commit that referenced this pull request Aug 17, 2018
…490)

 * Fixed: ThreadLocalScopeManager not passing finish_on_close to scope.

 * Added: Implementation for OpenTracer::Tracer

 * Added: OpenTracing::Tracer integration specs.
@delner delner added this to the 0.16.0 milestone Aug 23, 2018
delner added a commit that referenced this pull request Sep 13, 2018
…490)

 * Fixed: ThreadLocalScopeManager not passing finish_on_close to scope.

 * Added: Implementation for OpenTracer::Tracer

 * Added: OpenTracing::Tracer integration specs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Involves Datadog core libraries feature Involves a product feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants