Skip to content

Commit

Permalink
Adds EntrySplitter for use in w3c specs and secondary-sampling (#1193)
Browse files Browse the repository at this point in the history
This adds `EntrySplitter` to help handle complex formats such as W3C
trace-context efficiently. For example, this has flexible whitespace
rules to comply with standards like 'tracestate' which require retaining
leading whitespace.

See #693
  • Loading branch information
adriancole authored May 8, 2020
1 parent c0cfff3 commit 169d93a
Show file tree
Hide file tree
Showing 26 changed files with 798 additions and 39 deletions.
2 changes: 1 addition & 1 deletion .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ can be automatically added by running `./mvnw com.mycila:license-maven-plugin:fo

```
/**
* Copyright 2019 The OpenZipkin Authors
* Copyright 2020 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
Expand Down
3 changes: 3 additions & 0 deletions HACKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ the intent of the feature are often merged and released in days. If a
merged change isn't immediately released and it is of priority to you,
nag (make a comment) on your merged pull request until it is released.

It is best to review rationale documents such as [brave/RATIONALE.md]
prior to raising a pull request.

## How to experiment
Changes to Brave's code are best addressed by the feature requestor in a
pull request *after* discussing in an issue or on gitter. By discussing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*/
package brave.test.propagation;

import brave.internal.HexCodec;
import brave.internal.codec.HexCodec;
import brave.internal.Nullable;
import brave.propagation.Propagation;
import brave.propagation.Propagation.Getter;
Expand Down
49 changes: 48 additions & 1 deletion brave/RATIONALE.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,57 @@ thought as it would betray productivity and make this document unreadable.
Rationale here should be limited to impactful designs, and aspects non-obvious,
non-conventional or subtle.

## Public namespace
## Java conventions
Brave 4's public namespace is more defensive that the past, using a package
accessor design from [OkHttp](https://github.com/square/okhttp).

We only expose types public internally or after significant demand. This keeps
the api small and easier to manage when charting migration paths. Otherwise,
types are always package private.

Methods should only be marked public when they are intentional apis or
inheritance requires it. This practice prevents accidental dependence on
utilities.

### Why no private symbols? (methods and fields)
Brave is a library with embedded use cases, such as inside Java agents or
Android code.

For example, Android has a []hard limit on total methods in an application](https://developer.android.com/studio/build/multidex#avoid).
Fields marked private imply accessors in order to share state in the same
package. We routinely share state, such as sampling flag internals inside a
package. If we marked fields private, we'd count against that limit without
adding value.

Modifiers on fields and methods are distracting to read and increase the size
of the bytecode generated during compilation. By recovering the size otherwise
spent on private modifiers, we not only avoid hitting limits, but we are also
able to add more code with the same jar size.

For example, Brave 5.12 remains less than 250KiB, with no dependencies, all
features including deprecation bridges, and an embedded json serializer.

This means we do not support sharing our packages with third parties, but we
do support an "always share inside a package" in our repository. In other
words, we trust our developers to proceed with caution. In the first seven
years of project history, we have had no issues raised with this policy.

### Zero dependency policy
Brave is a telemetry library, which means it is used inside other low-level
libraries. We are unable to predict the version ranges of those libraries, and
attempting to do that would limit the applicability of Brave, which is an anti-
goal. Instead, we choose to use nothing except floor Java version features,
currently Java 6.

### Why `new NullPointerException("xxx == null")`
For public entry points, we eagerly check null and throw `NullPointerException`
with a message like "xxx == null". This is not a normal pre-condition, such as
argument validation, which you'd throw `IllegalArgumentException` for. What's
happening here is we are making debugging (literally NPEs are bugs) easier, by
not deferring to Java to raise the NPE. If we deferred, it could be confusing
which local was null, especially as deferring results in an exception with no
message.

## Stateful Span object
Brave 4 allows you to pass around a Span object which can report itself
to Zipkin when finished. This is better than using thread contexts in
Expand Down
2 changes: 1 addition & 1 deletion brave/src/main/java/brave/Tracing.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import brave.baggage.BaggageField;
import brave.handler.FinishedSpanHandler;
import brave.handler.MutableSpan;
import brave.internal.IpLiteral;
import brave.internal.codec.IpLiteral;
import brave.internal.Nullable;
import brave.internal.Platform;
import brave.internal.handler.NoopAwareFinishedSpanHandler;
Expand Down
4 changes: 2 additions & 2 deletions brave/src/main/java/brave/handler/MutableSpan.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@
import brave.ErrorParser;
import brave.Span.Kind;
import brave.SpanCustomizer;
import brave.internal.IpLiteral;
import brave.internal.codec.IpLiteral;
import brave.internal.Nullable;
import brave.propagation.TraceContext;
import java.lang.ref.WeakReference;
import java.util.ArrayList;

import static brave.internal.InternalPropagation.FLAG_DEBUG;
import static brave.internal.InternalPropagation.FLAG_SHARED;
import static brave.internal.JsonEscaper.jsonEscape;
import static brave.internal.codec.JsonEscaper.jsonEscape;

/**
* This represents a span except for its {@link TraceContext}. It is mutable, for late adjustments.
Expand Down
Loading

0 comments on commit 169d93a

Please sign in to comment.