Skip to content

[OpenTracing] Add skeleton code#462

Merged
delner merged 11 commits into
feature/opentracingfrom
feature/opentracing_skeleton
Jun 15, 2018
Merged

[OpenTracing] Add skeleton code#462
delner merged 11 commits into
feature/opentracingfrom
feature/opentracing_skeleton

Conversation

@delner

@delner delner commented Jun 13, 2018

Copy link
Copy Markdown
Contributor

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 palazzem and pawelchcki 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 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.

One small comment about using rspec filters.

Otherwise this PR looks like good starting point with OpenTracing.

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

if Datadog::OpenTracing.supported?

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.

Could this be moved into RSpec filter?

Copy link
Copy Markdown
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 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.

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

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

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.

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
Copy Markdown
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.

@@ -0,0 +1,6 @@
module Datadog
module OpenTracing
class Carrier < ::OpenTracing::Carrier

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.

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

require 'ddtrace/opentracing'
require 'ddtrace/opentracing/helper'

if Datadog::OpenTracing.supported?

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.

👍

@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 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
* 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
* 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
* 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
* 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
* 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
* 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
* 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

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