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.

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

OT API using Brave 4 #14

Merged
merged 17 commits into from
Feb 4, 2017
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 44 additions & 9 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.build.resourceEncoding>UTF-8</project.build.resourceEncoding>

<!-- default bytecode version for src/main -->
Copy link
Contributor

Choose a reason for hiding this comment

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

add or replace the root README with one that looks like the one from openzipkin/brave/brave (particularly the section about converting in-out from brave 3, as this is similar to converting in and out of opentracing)

<main.java.version>1.7</main.java.version>
<main.signature.artifact>java17</main.signature.artifact>

<!-- default bytecode version for src/test -->
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>1.8</maven.compiler.target>

<main.basedir>${project.basedir}</main.basedir>
</properties>

Expand Down Expand Up @@ -80,19 +88,21 @@
<dependencies>
<dependency>
<groupId>io.opentracing</groupId>
<artifactId>opentracing-impl</artifactId>
<artifactId>opentracing-api</artifactId>
<version>0.20.7</version>
</dependency>

<dependency>
<groupId>io.zipkin.brave</groupId>
<artifactId>brave-core</artifactId>
<version>3.17.0</version>
<artifactId>brave</artifactId>
<version>4.0.3</version>
</dependency>
<dependency>
<groupId>io.zipkin.brave</groupId>
<artifactId>brave-http</artifactId>
<version>3.17.0</version>
<artifactId>brave-core</artifactId>
<version>4.0.3</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure you need this dep anymore

</dependency>

<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
Expand All @@ -105,6 +115,12 @@
<version>1.10.19</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
<artifactId>assertj-core</artifactId>
<version>3.6.1</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand All @@ -113,9 +129,10 @@
<plugin>
<inherited>true</inherited>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.6.0</version>
<configuration>
<source>1.8</source>
<target>1.8</target>
<source>${maven.compiler.source}</source>
<target>${maven.compiler.target}</target>
</configuration>
<executions>
<execution>
Expand All @@ -125,12 +142,30 @@
<goal>compile</goal>
</goals>
<configuration>
<source>1.8</source>
<target>1.8</target>
<source>${maven.compiler.source}</source>
<target>${maven.compiler.target}</target>
</configuration>
</execution>
</executions>
</plugin>

<plugin>
<groupId>net.orfjackal.retrolambda</groupId>
<artifactId>retrolambda-maven-plugin</artifactId>
<version>2.4.0</version>
<executions>
<execution>
<goals>
<goal>process-main</goal>
</goals>
<configuration>
<target>${main.java.version}</target>
Copy link
Contributor

Choose a reason for hiding this comment

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

looks good

<fork>false</fork>
</configuration>
</execution>
</executions>
</plugin>

<!-- Language-level aside, make sure Java 9 types and methods aren't used -->
<plugin>
<groupId>org.codehaus.mojo</groupId>
Expand Down
177 changes: 177 additions & 0 deletions src/main/java/brave/features/opentracing/BraveSpan.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
/**
* Copyright 2016-2017 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
package brave.features.opentracing;

import io.opentracing.Span;
import io.opentracing.SpanContext;

import java.util.Map;

public class BraveSpan implements Span {
Copy link
Contributor

Choose a reason for hiding this comment

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

make type final with javadoc similar to BraveSpanContext


static BraveSpan wrap(brave.Span span) {
Copy link
Contributor

Choose a reason for hiding this comment

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

make this public and javadoc

/** Converts an existing {@linkplain brave.Span} for use in OpenTracing apis */

if (span == null) throw new NullPointerException("span == null");
return new BraveSpan(span);
}

final brave.Span unwrap() {
Copy link
Member Author

Choose a reason for hiding this comment

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

Wondering if this should be public?

Copy link
Contributor

Choose a reason for hiding this comment

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

yep it should

return delegate;
}

final brave.Span delegate;
final SpanContext context;

private BraveSpan(brave.Span delegate) {
this.delegate = delegate;
this.context = BraveSpanContext.wrap(delegate.context());
}

/**
* {@inheritDoc}
*/
@Override
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 javadoc are copied from the parent, maybe use

/**
 * {@inheritDoc}
 */

public SpanContext context() {
return context;
}

/**
* {@inheritDoc}
*/
@Override
public void finish() {
delegate.finish();
}

/**
* {@inheritDoc}
*/
@Override
public void finish(long finishMicros) {
delegate.finish(finishMicros);
}

/**
* {@inheritDoc}
*/
@Override
public void close() {
delegate.close();
}

/**
* {@inheritDoc}
*/
@Override
public Span setTag(String key, String value) {
delegate.tag(key, value);
return this;
}

/**
* {@inheritDoc}
*/
@Override
public Span setTag(String key, boolean value) {
return setTag(key, Boolean.toString(value));
}

/**
* {@inheritDoc}
*/
@Override
public Span setTag(String key, Number value) {
return setTag(key, value.toString());
}

/**
* {@inheritDoc}
*/
@Override
public Span log(Map<String, ?> fields) {
if (fields.isEmpty()) return this;
// in real life, do like zipkin-go-opentracing: "key1=value1 key2=value2"
return log(fields.toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd go ahead and make the utility function now

ex.

static String toAnnotation(Map<String, ?> fields) {
  // special-case the "event" field which is similar to the semantics of a zipkin annotation
  Object event = fields.get("event");
  if (event != null && fields.size() == 1) return event.toString();

  return joinOnEqualsSpace(fields);
}

static String joinOnEqualsSpace(Map<String, ?> fields) {
  if (fields.isEmpty()) return "";

  StringBuilder result = new StringBuilder();
  for (Iterator<Map.Entry<String, String>> i = fields.entrySet().iterator(); i.hasNext(); ) {
    Map.Entry<String, String> next = i.next();
    result.append(next.getKey()).append('=').append(next.getValue());
    if (i.hasNext()) result.append(' ');
  }
  return result.toString();
}

}

/**
* {@inheritDoc}
*/
@Override
public Span log(long timestampMicroseconds, Map<String, ?> fields) {
if (fields.isEmpty()) return this;
// in real life, do like zipkin-go-opentracing: "key1=value1 key2=value2"
return log(timestampMicroseconds, fields.toString());
}

/**
* {@inheritDoc}
*/
@Override
public Span log(String event) {
delegate.annotate(event);
return this;
}

/**
* {@inheritDoc}
*/
@Override
public Span log(long timestampMicroseconds, String event) {
delegate.annotate(timestampMicroseconds, event);
return this;
}

/**
* {@inheritDoc}
*/
@Override
public Span setBaggageItem(String key, String value) {
// brave does not support baggage
return this;
}

/**
* {@inheritDoc}
*/
@Override
public String getBaggageItem(String key) {
// brave does not support baggage
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe move this to javadoc instead of inheritdoc.

/**
  * Returns null as neither <a href="https://github.com/openzipkin/b3-propagation">B3</a>
  * nor Brave include baggage support.
  */
// OpenTracing could one day define a way to plug-in arbitrary baggage handling similar to how
// it has feature-specific apis like active-span

return null;
}

/**
* {@inheritDoc}
*/
@Override
public Span setOperationName(String operationName) {
delegate.name(operationName);
return this;
}

/**
* {@inheritDoc}
*/
@Override
public Span log(String eventName, Object ignored) {
return log(eventName);
}

/**
* {@inheritDoc}
*/
@Override
public Span log(long timestampMicroseconds, String eventName, Object ignored) {
return log(timestampMicroseconds, eventName);
}
}
Loading