Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[OpenTracing] Add skeleton code #462

Merged
merged 11 commits into from
Jun 15, 2018
Merged

[OpenTracing] Add skeleton code #462

merged 11 commits into from
Jun 15, 2018

Conversation

delner
Copy link
Contributor

@delner delner commented Jun 13, 2018

This pull request extends OpenTracing components in their most basic form, which are all no-ops. It should act as a starting point for adding code and tests as we begin to implement OpenTracing for ddtrace.

@delner delner added core Involves Datadog core libraries feature Involves a product feature labels Jun 13, 2018
@delner delner self-assigned this Jun 13, 2018
@delner delner requested review from pawelchcki and palazzem June 13, 2018 19:25
@delner delner force-pushed the feature/opentracing_skeleton branch from 950cd5b to b9666c9 Compare June 13, 2018 19:45
pawelchcki
pawelchcki requested changes Jun 14, 2018
View reviewed changes
Copy link
Contributor

@pawelchcki pawelchcki left a comment

Choose a reason for hiding this comment

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

One small comment about using rspec filters.

Otherwise this PR looks like good starting point with OpenTracing.

spec/ddtrace/opentracing/span_spec.rb Outdated
require 'ddtrace/opentracing'
require 'ddtrace/opentracing/helper'

if Datadog::OpenTracing.supported?
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this be moved into RSpec filter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. The next line below refers to a constant that is undefined if OpenTracing is not supported. We can only load these constants if Ruby >= 2.0.

Kyle-Verhoog
Kyle-Verhoog reviewed Jun 14, 2018
View reviewed changes
Copy link
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

Choose a reason for hiding this comment

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

Everything looks good opentracing-wise. Only the one potential action item with the naming consistency.

lib/ddtrace/opentracing.rb Outdated
@@ -0,0 +1,23 @@
module Datadog
# Namespace for ddtrace OpenTracing implementation
module OpenTracing
Copy link
Member

Choose a reason for hiding this comment

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

In both our python and go opentracing implementations we refer to the tracer and name the sub-module as 'opentracer'.

This is probably not a big deal, but we may wish to be consistent across our implementations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. For consistency with all other Datadog tracers, I'll change OpenTracing to OpenTracer.

lib/ddtrace/opentracing/carrier.rb Outdated
@@ -0,0 +1,6 @@
module Datadog
module OpenTracing
class Carrier < ::OpenTracing::Carrier
Copy link
Member

Choose a reason for hiding this comment

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

👍 the python opentracing library does not define a class for the carrier and is assumed to be a dict, this is better.

spec/ddtrace/opentracing/carrier_spec.rb Outdated
require 'ddtrace/opentracing'
require 'ddtrace/opentracing/helper'

if Datadog::OpenTracing.supported?
Copy link
Member

Choose a reason for hiding this comment

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

👍

Kyle-Verhoog
Kyle-Verhoog approved these changes Jun 14, 2018
View reviewed changes
Copy link
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍.

@delner delner merged commit 0e5560b into feature/opentracing Jun 15, 2018
@delner delner deleted the feature/opentracing_skeleton branch June 20, 2018 18:19
delner added a commit that referenced this pull request Jun 22, 2018
@delner
[OpenTracing] Add skeleton code (#462)
c9b98c6 
* Added: opentracing to gemspec.

* Added: Opentracing CI tasks.

* Added: Datadog::OpenTracer module.

* Added: Datadog::OpenTracer spec helpers.

* Added: Datadog::OpenTracer::Tracer.

* Added: Datadog::OpenTracer::Span.

* Added: Datadog::OpenTracer::SpanContext.

* Added: Datadog::OpenTracer::Scope.

* Added: Datadog::OpenTracer::ScopeManager.

* Added: Datadog::OpenTracer::Carrier.
delner added a commit that referenced this pull request Jul 10, 2018
@delner
[OpenTracing] Add skeleton code (#462)
7b3e5b3 
* Added: opentracing to gemspec.

* Added: Opentracing CI tasks.

* Added: Datadog::OpenTracer module.

* Added: Datadog::OpenTracer spec helpers.

* Added: Datadog::OpenTracer::Tracer.

* Added: Datadog::OpenTracer::Span.

* Added: Datadog::OpenTracer::SpanContext.

* Added: Datadog::OpenTracer::Scope.

* Added: Datadog::OpenTracer::ScopeManager.

* Added: Datadog::OpenTracer::Carrier.
delner added a commit that referenced this pull request Jul 16, 2018
@delner
[OpenTracing] Add skeleton code (#462)
ec70ff6 
* Added: opentracing to gemspec.

* Added: Opentracing CI tasks.

* Added: Datadog::OpenTracer module.

* Added: Datadog::OpenTracer spec helpers.

* Added: Datadog::OpenTracer::Tracer.

* Added: Datadog::OpenTracer::Span.

* Added: Datadog::OpenTracer::SpanContext.

* Added: Datadog::OpenTracer::Scope.

* Added: Datadog::OpenTracer::ScopeManager.

* Added: Datadog::OpenTracer::Carrier.
delner added a commit that referenced this pull request Jul 17, 2018
@delner
[OpenTracing] Add skeleton code (#462)
66b7747 
* Added: opentracing to gemspec.

* Added: Opentracing CI tasks.

* Added: Datadog::OpenTracer module.

* Added: Datadog::OpenTracer spec helpers.

* Added: Datadog::OpenTracer::Tracer.

* Added: Datadog::OpenTracer::Span.

* Added: Datadog::OpenTracer::SpanContext.

* Added: Datadog::OpenTracer::Scope.

* Added: Datadog::OpenTracer::ScopeManager.

* Added: Datadog::OpenTracer::Carrier.
delner added a commit that referenced this pull request Aug 9, 2018
@delner
[OpenTracing] Add skeleton code (#462)
7ba4100 
* Added: opentracing to gemspec.

* Added: Opentracing CI tasks.

* Added: Datadog::OpenTracer module.

* Added: Datadog::OpenTracer spec helpers.

* Added: Datadog::OpenTracer::Tracer.

* Added: Datadog::OpenTracer::Span.

* Added: Datadog::OpenTracer::SpanContext.

* Added: Datadog::OpenTracer::Scope.

* Added: Datadog::OpenTracer::ScopeManager.

* Added: Datadog::OpenTracer::Carrier.
delner added a commit that referenced this pull request Aug 17, 2018
@delner
[OpenTracing] Add skeleton code (#462)
3ccaced 
* Added: opentracing to gemspec.

* Added: Opentracing CI tasks.

* Added: Datadog::OpenTracer module.

* Added: Datadog::OpenTracer spec helpers.

* Added: Datadog::OpenTracer::Tracer.

* Added: Datadog::OpenTracer::Span.

* Added: Datadog::OpenTracer::SpanContext.

* Added: Datadog::OpenTracer::Scope.

* Added: Datadog::OpenTracer::ScopeManager.

* Added: Datadog::OpenTracer::Carrier.
@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
@delner
[OpenTracing] Add skeleton code (#462)
0f5142e 
* Added: opentracing to gemspec.

* Added: Opentracing CI tasks.

* Added: Datadog::OpenTracer module.

* Added: Datadog::OpenTracer spec helpers.

* Added: Datadog::OpenTracer::Tracer.

* Added: Datadog::OpenTracer::Span.

* Added: Datadog::OpenTracer::SpanContext.

* Added: Datadog::OpenTracer::Scope.

* Added: Datadog::OpenTracer::ScopeManager.

* Added: Datadog::OpenTracer::Carrier.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kyle-Verhoog Kyle-Verhoog Kyle-Verhoog approved these changes

@pawelchcki pawelchcki pawelchcki approved these changes

@palazzem palazzem Awaiting requested review from palazzem

Assignees

@delner delner

Labels
core Involves Datadog core libraries feature Involves a product feature
Projects
None yet
Milestone
0.16.0
Development

Successfully merging this pull request may close these issues.
3 participants
@delner @pawelchcki @Kyle-Verhoog