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

Add baggage restriction manager #210

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion idl
Submodule idl updated from 23f33f to 56539e
10 changes: 9 additions & 1 deletion jaeger-core/src/main/java/com/uber/jaeger/Configuration.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import com.uber.jaeger.samplers.Sampler;
import com.uber.jaeger.senders.Sender;
import com.uber.jaeger.senders.UdpSender;

import java.net.URISyntaxException;
import java.text.NumberFormat;
import java.text.ParseException;
import java.util.HashMap;
Expand Down Expand Up @@ -258,7 +260,13 @@ private Sampler createSampler(String serviceName, Metrics metrics) {
if (samplerType.equals(RemoteControlledSampler.TYPE)) {
Sampler initialSampler = new ProbabilisticSampler(samplerParam.doubleValue());

HttpSamplingManager manager = new HttpSamplingManager(hostPort);
HttpSamplingManager manager = null;
try {
manager = new HttpSamplingManager(hostPort);
} catch (URISyntaxException e) {
log.error("JAEGER_SAMPLER_MANAGER_HOST_PORT '" + hostPort + "' is not a valid hostPort", e);
throw new IllegalArgumentException("JAEGER_SAMPLER_MANAGER_HOST_PORT must be valid");
}

return new RemoteControlledSampler(serviceName, manager, initialSampler, metrics);
}
Expand Down
29 changes: 17 additions & 12 deletions jaeger-core/src/main/java/com/uber/jaeger/Span.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

package com.uber.jaeger;

import com.uber.jaeger.baggage.SanitizedBaggage;
import io.opentracing.tag.Tags;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -117,21 +118,25 @@ public List<LogData> getLogs() {
@Override
public Span setBaggageItem(String key, String value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

setBaggageItem should check that key is not null or empty and value isn't null.

synchronized (this) {
// TODO emit a metric whenever baggage is updated
String prevItem = this.getBaggageItem(key);
this.context = this.context.withBaggageItem(key, value);
if (key == null || value == null) {
return this;
}
String prevValue = this.getBaggageItem(key);
SanitizedBaggage baggage = this.tracer.getBaggageRestrictionManager().sanitizeBaggage(key, value, prevValue);
if (!baggage.isValid()) {
this.tracer.getMetrics().baggageUpdateFailure.inc(1);
return this;
}
if (baggage.isSanitized()) {
this.tracer.getMetrics().baggageSanitize.inc(1);
}
this.context = this.context.withBaggageItem(key, baggage.getSanitizedValue());
this.tracer.getMetrics().baggageUpdateSuccess.inc(1);
if (context.isSampled()) {
Map<String, String> fields = new HashMap<String, String>();
fields.put("event", "baggage");
fields.put("key", key);
fields.put("value", value);
if (prevItem != null) {
fields.put("override", "true");
}
return this.log(fields);
return this.log(baggage.getFields());
}
return this;
}
return this;
}

@Override
Expand Down
19 changes: 17 additions & 2 deletions jaeger-core/src/main/java/com/uber/jaeger/Tracer.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
package com.uber.jaeger;

import com.uber.jaeger.Constants;
import com.uber.jaeger.baggage.BaggageRestrictionManager;
import com.uber.jaeger.baggage.DefaultBaggageRestrictionManager;
import com.uber.jaeger.exceptions.UnsupportedFormatException;
import com.uber.jaeger.metrics.Metrics;
import com.uber.jaeger.metrics.NullStatsReporter;
Expand Down Expand Up @@ -74,6 +76,7 @@ public class Tracer implements io.opentracing.Tracer {
private final Map<String, ?> tags;
private final boolean zipkinSharedRpcSpan;
private final ActiveSpanSource activeSpanSource;
private final BaggageRestrictionManager baggageRestrictionManager;

private Tracer(
String serviceName,
Expand All @@ -84,7 +87,8 @@ private Tracer(
Metrics metrics,
Map<String, Object> tags,
boolean zipkinSharedRpcSpan,
ActiveSpanSource activeSpanSource) {
ActiveSpanSource activeSpanSource,
BaggageRestrictionManager baggageRestrictionManager) {
this.serviceName = serviceName;
this.reporter = reporter;
this.sampler = sampler;
Expand All @@ -93,6 +97,7 @@ private Tracer(
this.metrics = metrics;
this.zipkinSharedRpcSpan = zipkinSharedRpcSpan;
this.activeSpanSource = activeSpanSource;
this.baggageRestrictionManager = baggageRestrictionManager;

this.version = loadVersion();

Expand Down Expand Up @@ -140,6 +145,10 @@ Reporter getReporter() {
return reporter;
}

BaggageRestrictionManager getBaggageRestrictionManager() {
return baggageRestrictionManager;
}

void reportSpan(Span span) {
reporter.report(span);
metrics.spansFinished.inc(1);
Expand Down Expand Up @@ -448,6 +457,7 @@ public static final class Builder {
private Map<String, Object> tags = new HashMap<String, Object>();
private boolean zipkinSharedRpcSpan;
private ActiveSpanSource activeSpanSource = new ThreadLocalActiveSpanSource();
private BaggageRestrictionManager baggageRestrictionManager = new DefaultBaggageRestrictionManager();

public Builder(String serviceName, Reporter reporter, Sampler sampler) {
if (serviceName == null || serviceName.trim().length() == 0) {
Expand Down Expand Up @@ -524,9 +534,14 @@ public Builder withTags(Map<String, String> tags) {
return this;
}

public Builder withBaggageRestrictionManager(BaggageRestrictionManager baggageRestrictionManager) {
this.baggageRestrictionManager = baggageRestrictionManager;
return this;
}

public Tracer build() {
return new Tracer(this.serviceName, reporter, sampler, registry, clock, metrics, tags,
zipkinSharedRpcSpan, activeSpanSource);
zipkinSharedRpcSpan, activeSpanSource, baggageRestrictionManager);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* 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.baggage;

import java.util.HashMap;
import java.util.Map;

import lombok.AccessLevel;
import lombok.Getter;

public abstract class BaggageRestrictionManager {
static final int DEFAULT_MAX_VALUE_LENGTH = 2048;
static final SanitizedBaggage INVALID_BAGGAGE = SanitizedBaggage.of(false, null, null, null, false);

public abstract SanitizedBaggage sanitizeBaggage(String key, String value, String prevValue);

SanitizedBaggage sanitizeBaggage(String key, String value, String prevValue, int maxValueLength) {
Copy link
Member

Choose a reason for hiding this comment

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

What is the semantic difference between truncated and sanitized? What other modification could be done aside from truncating?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we could be normalizing the key ala: https://github.com/uber/jaeger-client-go/blob/master/span.go#L259 or we might have to obfuscate the value, etc.

Copy link
Member

Choose a reason for hiding this comment

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

Doesn't make sense to normalize the key since the user will be expecting to get back the same key that they put in. We may encode the key when serialised, but not here.

TruncatedBaggage baggage = truncateBaggage(value, maxValueLength);
Map<String, String> fields = new HashMap<String, String>();
fields.put("event", "baggage");
fields.put("key", key);
fields.put("value", baggage.getTruncatedValue());
if (baggage.isTruncated()) {
fields.put("truncated", "true");
}
if (prevValue != null) {
fields.put("override", "true");
}
Copy link
Member

Choose a reason for hiding this comment

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

this is an abstraction leak. Why should baggage manager care about log fields? Also, because there is no span context here, we're creating the map every time even though the span may be unsampled and logs will be no-op.

return SanitizedBaggage.of(true, key, baggage.getTruncatedValue(), fields,
baggage.isTruncated());
}

TruncatedBaggage truncateBaggage(String value, int maxValueLength) {
if (value.length() > maxValueLength) {
return new TruncatedBaggage(value.substring(0, maxValueLength), true);
}
return new TruncatedBaggage(value, false);
}

@Getter(AccessLevel.PACKAGE)
class TruncatedBaggage {
Copy link
Member

Choose a reason for hiding this comment

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

we should really consider the performance impact of all these helper classes. Conceptually we're doing this:

func setBaggageItem(key, value) {
    if !mgr.allows(key) return
    value2 = mgr.truncate(value)
    span.baggage.put(key, value2)
    if span.sampled {
       logFields = ...
       if value.length() != value2.length() {
         logFields['truncated'] = true
       }
       span.log(logFields)
    }
}

Aside from logFields there are no memory allocations here.

final String truncatedValue;
final boolean truncated;

TruncatedBaggage(String truncatedValue, boolean truncated) {
this.truncatedValue = truncatedValue;
this.truncated = truncated;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* 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.baggage;

import com.uber.jaeger.baggage.http.BaggageRestriction;
import com.uber.jaeger.exceptions.BaggageRestrictionException;

import java.util.List;

public interface BaggageRestrictionProxy {
Copy link
Member

Choose a reason for hiding this comment

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

baggage restriction manager proxy

List<BaggageRestriction> getBaggageRestrictions(String serviceName)
throws BaggageRestrictionException;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* 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.baggage;

public class DefaultBaggageRestrictionManager extends BaggageRestrictionManager {

@Override
public SanitizedBaggage sanitizeBaggage(String key, String value, String prevValue) {
return sanitizeBaggage(key, value, prevValue, DEFAULT_MAX_VALUE_LENGTH);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* 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.baggage;

import static com.uber.jaeger.utils.Utils.makeGetRequest;

import com.google.gson.Gson;
import com.google.gson.JsonSyntaxException;
import com.google.gson.reflect.TypeToken;
import com.uber.jaeger.baggage.http.BaggageRestriction;
import com.uber.jaeger.exceptions.BaggageRestrictionException;

import java.lang.reflect.Type;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.List;

import org.apache.http.client.utils.URIBuilder;

public class HttpBaggageRestrictionProxy implements BaggageRestrictionProxy {
private static final String DEFAULT_HOST_PORT = "localhost:5778";
private final URIBuilder builder;
private final Gson gson = new Gson();

public HttpBaggageRestrictionProxy(String hostPort) throws URISyntaxException {
hostPort = hostPort != null ? hostPort : DEFAULT_HOST_PORT;
this.builder = new URIBuilder("http://" + hostPort).setPath("/baggageRestrictions");
}

List<BaggageRestriction> parseJson(String json) throws BaggageRestrictionException {
try {
Type listType = new TypeToken<ArrayList<BaggageRestriction>>(){}.getType();
return gson.fromJson(json, listType);
} catch (JsonSyntaxException e) {
throw new BaggageRestrictionException("Cannot deserialize json", e);
}
}

@Override
public List<BaggageRestriction> getBaggageRestrictions(String serviceName)
throws BaggageRestrictionException {
String jsonString;
try {
jsonString =
makeGetRequest(builder.setParameter("service", serviceName).build());
Copy link
Member

Choose a reason for hiding this comment

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

is the url builder really reusable like that? Especially from multiple threads?

} catch (Exception e) {
throw new BaggageRestrictionException(
"http call to get baggage restriction from local agent failed.", e);
}
return parseJson(jsonString);
}
}
Loading