Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Initial implementation of a TracerResolver for Jaeger #175

Merged
merged 7 commits into from
May 14, 2017
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ ext.jerseyVersion = '2.22.2'
ext.slf4jVersion = '1.7.16'
ext.apacheHttpComponentsVersion = '4.1.2'
ext.gsonVersion = '2.8.0'
ext.tracerResolverVersion = '0.1.0'

ext.junitVersion = '4.12'
ext.mockitoVersion = '2.2.28'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ public void run() {
flushInterval);
}

public int getMaxQueueSize() {
return maxQueueSize;
}

@Override
public void report(Span span) {
// Its better to drop spans, than to block here
Expand Down
35 changes: 35 additions & 0 deletions jaeger-tracerresolver/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Jaeger Tracer Resolver
Copy link
Contributor

@mabn mabn May 10, 2017

Choose a reason for hiding this comment

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

It would be good to have here a short comment describing what it is


This module provides a Jaeger implementation for the [TracerResolver](https://github.com/opentracing-contrib/java-tracerresolver). This mechanism provides a vendor neutral approach for obtaining a `Tracer` using the JDK
`ServiceLoader`.


## Maven Dependency
```xml
<dependency>
<groupId>com.uber.jaeger</groupId>
<artifactId>jaeger-tracerresolver</artifactId>
<version>$jaegerVersion</version>
</dependency>
```

## Usage

```java
Tracer tracer = TracerResolver.resolveTracer();
```

## Configuration Options

The following configuration properties can be provided either as an environment variable or system property.
Copy link
Member

Choose a reason for hiding this comment

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

Should we move these to the README in core as well? We can link to that file from here.

Right now the main README only suggests

Configuration config = new Configuration("myServiceName", null, null);

for production. I would add a sub-section there "Configuration via Environment" with what we have here. And it would also make sense to add yet another subsection that it's possible to init via TracerLoader, and link to here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good - I'll have to look at this next week.

The system property will override an environment variable for the same property name.

Property | Default | Description
--- | --- | ---
JAEGER_SERVICE_NAME | _none_ | The service name (must be defined)
JAEGER_AGENT_UDP_MAX_PACKET_SIZE | 65000 | The maximum packet size when communicating with the agent via UDP
Copy link
Member

Choose a reason for hiding this comment

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

These default values can get quickly outdated.

Copy link
Member

Choose a reason for hiding this comment

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

agreed - better to just say required/optional. The defaults will be applied by Configuration class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok will update

JAEGER_AGENT_UDP_HOST | localhost | The hostname for communicating with agent via UDP
JAEGER_AGENT_UDP_PORT | 6831 | The port for communicating with agent via UDP
JAEGER_REPORTER_MAX_QUEUE_SIZE | 1000 | The reporter's maximum queue size
JAEGER_REPORTER_FLUSH_INTERVAL | 500 | The reporter's flush interval (ms)

34 changes: 34 additions & 0 deletions jaeger-tracerresolver/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
description = 'TracerResolver implementation for Jaeger Tracer'

dependencies {
compile project(':jaeger-core')
compile group: 'io.opentracing.contrib', name: 'opentracing-tracerresolver', version: tracerResolverVersion

testCompile group: 'junit', name: 'junit', version: junitVersion

signature 'org.codehaus.mojo.signature:java16:1.1@signature'
}

sourceSets {
Copy link
Member

Choose a reason for hiding this comment

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

Can be this boilerplate avoided? I think it's redundant as we are sticking to default file hierarchy.

I know this is repeated over and over it other modules, but should be used only in places where it's required to override a default conventions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As above, this should probably be discussed as a separate issue and applied to all artifacts.

Copy link
Member

Choose a reason for hiding this comment

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

The previous maybe yes. This can be done independently it does not affect other modules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep that works.

main {
java {
srcDir 'src/main/java'
}
resources {
srcDir 'src/main/resources'
}
}

test {
java {
srcDir 'src/test/java'
}
}
}

jar {
from sourceSets.main.output
manifest {
attributes('Implementation-Title': 'jaeger-tracerresolver', 'Implementation-Version': project.version)
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't be this added to allProjects in root script?

Copy link
Member

Choose a reason for hiding this comment

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

It's repeated in every module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If its possible then sounds like a good idea, but outside the remit of this PR.

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
* Copyright (c) 2017, Uber Technologies, Inc
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package com.uber.jaeger.tracerresolver;

import com.uber.jaeger.Tracer;
import com.uber.jaeger.metrics.Metrics;
import com.uber.jaeger.metrics.NullStatsReporter;
import com.uber.jaeger.reporters.RemoteReporter;
import com.uber.jaeger.reporters.Reporter;
import com.uber.jaeger.samplers.ConstSampler;
import com.uber.jaeger.samplers.Sampler;
import com.uber.jaeger.senders.Sender;
import com.uber.jaeger.senders.UdpSender;

import io.opentracing.contrib.tracerresolver.TracerResolver;
import lombok.extern.slf4j.Slf4j;

@Slf4j
public class JaegerTracerResolver extends TracerResolver {

static final String JAEGER_PREFIX = "JAEGER_";
Copy link
Member

@pavolloffay pavolloffay May 10, 2017

Choose a reason for hiding this comment

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

these should be public and with javadoc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are only used by the resolver.

Copy link
Member

Choose a reason for hiding this comment

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

They are used outside of this module. Consumers set these when starting instrumented app, so are public. Somebody could use them programmatically. Readmes might get outdated, therefore having javadoc is a plus.

static final String JAEGER_AGENT_UDP_MAX_PACKET_SIZE = JAEGER_PREFIX + "AGENT_UDP_MAX_PACKET_SIZE";
static final String JAEGER_AGENT_UDP_PORT = JAEGER_PREFIX + "AGENT_UDP_PORT";
static final String JAEGER_AGENT_UDP_HOST = JAEGER_PREFIX + "AGENT_UDP_HOST";
static final String JAEGER_REPORTER_MAX_QUEUE_SIZE = JAEGER_PREFIX + "REPORTER_MAX_QUEUE_SIZE";
static final String JAEGER_REPORTER_FLUSH_INTERVAL = JAEGER_PREFIX + "REPORTER_FLUSH_INTERVAL";
static final String JAEGER_SERVICE_NAME = JAEGER_PREFIX + "SERVICE_NAME";

static final int DEFAULT_REPORTER_FLUSH_INTERVAL = 500;
static final int DEFAULT_REPORTER_MAX_QUEUE_SIZE = 1000;

@Override
protected io.opentracing.Tracer resolve() {
String serviceName = getProperty(JAEGER_SERVICE_NAME);

return new Tracer.Builder(serviceName, getReporter(), getSampler()).build();
}

protected static Reporter getReporter() {
return new RemoteReporter(getSender(),
Copy link
Member

Choose a reason for hiding this comment

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

Could we avoid writing this code and instead use the Configuration class to create the tracer? Here we just need to set the parameters on the respective Config classes, but leave the init logic to one place in the main config.

getPropertyAsInt(JAEGER_REPORTER_FLUSH_INTERVAL, DEFAULT_REPORTER_FLUSH_INTERVAL),
getPropertyAsInt(JAEGER_REPORTER_MAX_QUEUE_SIZE, DEFAULT_REPORTER_MAX_QUEUE_SIZE),
getMetrics());
}

protected static Sender getSender() {
return new UdpSender(getProperty(JAEGER_AGENT_UDP_HOST),
getPropertyAsInt(JAEGER_AGENT_UDP_PORT, 0),
getPropertyAsInt(JAEGER_AGENT_UDP_MAX_PACKET_SIZE, 0));
}

protected static Metrics getMetrics() {
// TODO: Support other stats reporters
return Metrics.fromStatsReporter(new NullStatsReporter());
}

protected static Sampler getSampler() {
// TODO: Support other samplers
return new ConstSampler(true);
}

private static String getProperty(String name) {
return System.getProperty(name, System.getenv(name));
}

private static int getPropertyAsInt(String name, int def) {
String value = getProperty(name);
if (value != null) {
try {
return Integer.parseInt(value);
} catch (NumberFormatException e) {
log.error("Failed to parse integer for property '" + name + "' with value '" + value + "'", e);
}
}
return def;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
com.uber.jaeger.tracerresolver.JaegerTracerResolver
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/*
* Copyright (c) 2017, Uber Technologies, Inc
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package com.uber.jaeger.tracerresolver;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;

import com.uber.jaeger.reporters.RemoteReporter;
import com.uber.jaeger.reporters.Reporter;
import com.uber.jaeger.samplers.ConstSampler;
import com.uber.jaeger.senders.UdpSender;

import io.opentracing.Tracer;
import io.opentracing.contrib.tracerresolver.TracerResolver;

import java.util.Enumeration;

import org.junit.Before;
import org.junit.Test;

public class JaegerTracerResolverTest {

@Before
public void clearProperties() {
Enumeration<?> iter = System.getProperties().propertyNames();
while (iter.hasMoreElements()) {
String propName = (String)iter.nextElement();
if (propName.startsWith(JaegerTracerResolver.JAEGER_PREFIX)) {
System.clearProperty(propName);
Copy link
Member

Choose a reason for hiding this comment

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

nit: clear only properties set by this test

}
}
}

@Test
public void testResolveTracerNoServiceName() {
assertNull(TracerResolver.resolveTracer());
}

@Test
public void testResolveTracerDefault() {
System.setProperty(JaegerTracerResolver.JAEGER_SERVICE_NAME, "MyService");
Tracer tracer = TracerResolver.resolveTracer();
assertNotNull(tracer);
assertTrue(tracer instanceof com.uber.jaeger.Tracer);
}

@Test
public void testSamplerConst() {
assertTrue(JaegerTracerResolver.getSampler() instanceof ConstSampler);
}

@Test
public void testSender() {
assertTrue(JaegerTracerResolver.getSender() instanceof UdpSender);
}

@Test
public void testMetrics() {
assertNotNull(JaegerTracerResolver.getMetrics());
}

@Test
public void testReporter() {
System.setProperty(JaegerTracerResolver.JAEGER_REPORTER_MAX_QUEUE_SIZE, "10");
Reporter reporter = JaegerTracerResolver.getReporter();
assertTrue(reporter instanceof RemoteReporter);
assertEquals(10, ((RemoteReporter)reporter).getMaxQueueSize());
}

@Test
public void testReporterInvalidQueueSize() {
System.setProperty(JaegerTracerResolver.JAEGER_REPORTER_MAX_QUEUE_SIZE, "X");
Reporter reporter = JaegerTracerResolver.getReporter();
assertTrue(reporter instanceof RemoteReporter);
assertEquals(JaegerTracerResolver.DEFAULT_REPORTER_MAX_QUEUE_SIZE, ((RemoteReporter)reporter).getMaxQueueSize());
}

}
1 change: 1 addition & 0 deletions settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ include 'jaeger-zipkin'
include 'jaeger-crossdock'
include 'jaeger-thrift'
include 'jaeger-apachehttpclient'
include 'jaeger-tracerresolver'