Simplify default tags handling

Remove special case of tags beng empty - it is unlikely to improve
performance but it complicated code unnecessarily.
This commit is contained in:
Nikolay Martynov 2018-07-30 17:31:42 -04:00
parent 4c88e1a0a8
commit 2c0702f278
2 changed files with 36 additions and 40 deletions

View File

@ -56,7 +56,7 @@ public class DDTracer implements io.opentracing.Tracer {
final ContextualScopeManager scopeManager = new ContextualScopeManager(); final ContextualScopeManager scopeManager = new ContextualScopeManager();
/** A set of tags that are added to every span */ /** A set of tags that are added to every span */
private final Map<String, String> spanTags; private final Map<String, String> defaultSpanTags;
/** A configured mapping of service names to update with new values */ /** A configured mapping of service names to update with new values */
private final Map<String, String> serviceNameMappings; private final Map<String, String> serviceNameMappings;
@ -117,7 +117,7 @@ public class DDTracer implements io.opentracing.Tracer {
this.writer = writer; this.writer = writer;
this.writer.start(); this.writer.start();
this.sampler = sampler; this.sampler = sampler;
this.spanTags = defaultSpanTags; this.defaultSpanTags = defaultSpanTags;
this.serviceNameMappings = serviceNameMappings; this.serviceNameMappings = serviceNameMappings;
try { try {
@ -126,7 +126,7 @@ public class DDTracer implements io.opentracing.Tracer {
new Thread() { new Thread() {
@Override @Override
public void run() { public void run() {
DDTracer.this.close(); close();
} }
}); });
} catch (final IllegalStateException ex) { } catch (final IllegalStateException ex) {
@ -287,8 +287,8 @@ public class DDTracer implements io.opentracing.Tracer {
} }
} }
traceCount.incrementAndGet(); traceCount.incrementAndGet();
if (!writtenTrace.isEmpty() && this.sampler.sample(writtenTrace.get(0))) { if (!writtenTrace.isEmpty() && sampler.sample(writtenTrace.get(0))) {
this.writer.write(writtenTrace); writer.write(writtenTrace);
} }
} }
@ -301,14 +301,14 @@ public class DDTracer implements io.opentracing.Tracer {
public String toString() { public String toString() {
return "DDTracer-" return "DDTracer-"
+ Integer.toHexString(hashCode()) + Integer.toHexString(hashCode())
+ "{ service-name=" + "{ serviceName="
+ serviceName + serviceName
+ ", writer=" + ", writer="
+ writer + writer
+ ", sampler=" + ", sampler="
+ sampler + sampler
+ ", tags=" + ", defaultSpanTags="
+ spanTags + defaultSpanTags
+ '}'; + '}';
} }
@ -333,10 +333,7 @@ public class DDTracer implements io.opentracing.Tracer {
private final String operationName; private final String operationName;
// Builder attributes // Builder attributes
private Map<String, Object> tags = private Map<String, Object> tags = new HashMap<String, Object>(defaultSpanTags);
spanTags.isEmpty()
? Collections.<String, Object>emptyMap()
: new HashMap<String, Object>(spanTags);
private long timestampMicro; private long timestampMicro;
private SpanContext parent; private SpanContext parent;
private String serviceName; private String serviceName;
@ -352,14 +349,14 @@ public class DDTracer implements io.opentracing.Tracer {
@Override @Override
public SpanBuilder ignoreActiveSpan() { public SpanBuilder ignoreActiveSpan() {
this.ignoreScope = true; ignoreScope = true;
return this; return this;
} }
private DDSpan startSpan() { private DDSpan startSpan() {
final DDSpan span = new DDSpan(this.timestampMicro, buildSpanContext()); final DDSpan span = new DDSpan(timestampMicro, buildSpanContext());
if (DDTracer.this.sampler instanceof RateByServiceSampler) { if (sampler instanceof RateByServiceSampler) {
((RateByServiceSampler) DDTracer.this.sampler).initializeSamplingPriority(span); ((RateByServiceSampler) sampler).initializeSamplingPriority(span);
} }
return span; return span;
} }
@ -402,7 +399,7 @@ public class DDTracer implements io.opentracing.Tracer {
@Override @Override
public DDSpanBuilder withStartTimestamp(final long timestampMicroseconds) { public DDSpanBuilder withStartTimestamp(final long timestampMicroseconds) {
this.timestampMicro = timestampMicroseconds; timestampMicro = timestampMicroseconds;
return this; return this;
} }
@ -417,7 +414,7 @@ public class DDTracer implements io.opentracing.Tracer {
} }
public DDSpanBuilder withErrorFlag() { public DDSpanBuilder withErrorFlag() {
this.errorFlag = true; errorFlag = true;
return this; return this;
} }
@ -440,7 +437,7 @@ public class DDTracer implements io.opentracing.Tracer {
@Override @Override
public DDSpanBuilder asChildOf(final SpanContext spanContext) { public DDSpanBuilder asChildOf(final SpanContext spanContext) {
this.parent = spanContext; parent = spanContext;
return this; return this;
} }
@ -453,14 +450,9 @@ public class DDTracer implements io.opentracing.Tracer {
// Private methods // Private methods
private DDSpanBuilder withTag(final String tag, final Object value) { private DDSpanBuilder withTag(final String tag, final Object value) {
if (value == null || (value instanceof String && ((String) value).isEmpty())) { if (value == null || (value instanceof String && ((String) value).isEmpty())) {
if (this.tags.containsKey(tag)) { tags.remove(tag);
this.tags.remove(tag);
}
} else { } else {
if (this.tags.isEmpty()) { tags.put(tag, value);
this.tags = new HashMap<>();
}
this.tags.put(tag, value);
} }
return this; return this;
} }
@ -486,11 +478,13 @@ public class DDTracer implements io.opentracing.Tracer {
final int samplingPriority; final int samplingPriority;
final DDSpanContext context; final DDSpanContext context;
SpanContext parentContext = this.parent; SpanContext parentContext = parent;
if (parentContext == null && !ignoreScope) { if (parentContext == null && !ignoreScope) {
// use the Scope as parent unless overridden or ignored. // use the Scope as parent unless overridden or ignored.
final Scope scope = scopeManager.active(); final Scope scope = scopeManager.active();
if (scope != null) parentContext = scope.span().context(); if (scope != null) {
parentContext = scope.span().context();
}
} }
// Propagate internal trace // Propagate internal trace
@ -501,8 +495,12 @@ public class DDTracer implements io.opentracing.Tracer {
baggage = ddsc.getBaggageItems(); baggage = ddsc.getBaggageItems();
parentTrace = ddsc.getTrace(); parentTrace = ddsc.getTrace();
samplingPriority = PrioritySampling.UNSET; samplingPriority = PrioritySampling.UNSET;
if (this.serviceName == null) this.serviceName = ddsc.getServiceName(); if (serviceName == null) {
if (this.spanType == null) this.spanType = ddsc.getSpanType(); serviceName = ddsc.getServiceName();
}
if (spanType == null) {
spanType = ddsc.getSpanType();
}
// Propagate external trace // Propagate external trace
} else if (parentContext instanceof ExtractedContext) { } else if (parentContext instanceof ExtractedContext) {
@ -510,8 +508,8 @@ public class DDTracer implements io.opentracing.Tracer {
traceId = ddsc.getTraceId(); traceId = ddsc.getTraceId();
parentSpanId = ddsc.getSpanId(); parentSpanId = ddsc.getSpanId();
baggage = ddsc.getBaggage(); baggage = ddsc.getBaggage();
if (this.tags.isEmpty() && !ddsc.getTags().isEmpty()) { if (tags.isEmpty() && !ddsc.getTags().isEmpty()) {
this.tags = new HashMap<>(); tags = new HashMap<>();
} }
if (!ddsc.getTags().isEmpty()) { if (!ddsc.getTags().isEmpty()) {
tags.putAll(ddsc.getTags()); tags.putAll(ddsc.getTags());
@ -532,8 +530,7 @@ public class DDTracer implements io.opentracing.Tracer {
serviceName = DDTracer.this.serviceName; serviceName = DDTracer.this.serviceName;
} }
final String operationName = final String operationName = this.operationName != null ? this.operationName : resourceName;
this.operationName != null ? this.operationName : this.resourceName;
// some attributes are inherited from the parent // some attributes are inherited from the parent
context = context =
@ -543,17 +540,17 @@ public class DDTracer implements io.opentracing.Tracer {
parentSpanId, parentSpanId,
serviceName, serviceName,
operationName, operationName,
this.resourceName, resourceName,
samplingPriority, samplingPriority,
baggage, baggage,
errorFlag, errorFlag,
spanType, spanType,
this.tags, tags,
parentTrace, parentTrace,
DDTracer.this); DDTracer.this);
// Apply Decorators to handle any tags that may have been set via the builder. // Apply Decorators to handle any tags that may have been set via the builder.
for (final Map.Entry<String, Object> tag : this.tags.entrySet()) { for (final Map.Entry<String, Object> tag : tags.entrySet()) {
if (tag.getValue() == null) { if (tag.getValue() == null) {
context.setTag(tag.getKey(), null); context.setTag(tag.getKey(), null);
continue; continue;
@ -562,8 +559,7 @@ public class DDTracer implements io.opentracing.Tracer {
boolean addTag = true; boolean addTag = true;
// Call decorators // Call decorators
final List<AbstractDecorator> decorators = final List<AbstractDecorator> decorators = getSpanContextDecorators(tag.getKey());
DDTracer.this.getSpanContextDecorators(tag.getKey());
if (decorators != null) { if (decorators != null) {
for (final AbstractDecorator decorator : decorators) { for (final AbstractDecorator decorator : decorators) {
try { try {

View File

@ -121,7 +121,7 @@ class DDTraceConfigTest extends Specification {
def taggedHeaders = tracer.registry.codecs.values().first().taggedHeaders def taggedHeaders = tracer.registry.codecs.values().first().taggedHeaders
then: then:
tracer.spanTags == map tracer.defaultSpanTags == map
tracer.serviceNameMappings == map tracer.serviceNameMappings == map
taggedHeaders == map taggedHeaders == map