Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Reads and writes gRPC trace and tags binary metadata #688

Merged
merged 1 commit into from
Jun 14, 2018

Conversation

codefromthecrypt
Copy link
Member

@codefromthecrypt codefromthecrypt commented Apr 11, 2018

gRPC Propagation Format (Census interop)

gRPC defines a binary encoded propagation format which is implemented
by OpenCensus instrumentation. When this is
the case, incoming requests will have two metadata keys "grpc-trace-bin"
and "grpc-tags-bin".

When enabled, this component can extract trace contexts from these
metadata and also write the same keys on outgoing calls. This allows
transparent interop when both census and brave report data to the same
tracing system.

To enable this feature, set grpcPropagationFormatEnabled which is off
by default:

grpcTracing = GrpcTracing.newBuilder(tracing)
                         .grpcPropagationFormatEnabled(true).build();

Warning: the format of both "grpc-trace-bin" and "grpc-tags-bin" are
version 0. As such, consider this feature experimental.

notes:

This allows interop with gRPC services that don't use B3 (for example,
the default census module). This also acknowledges that the OpenCensus
binary format is fairly contained within gRPC, so doesn't need to become
a top-level propagation format, for example used with non-gRPC services
as yet.

Redundantly encoding this format has some cost to it, notable 14
character header name "grpc-trace-bin" and the 38 character base64
encoded binary context value.

Note that this only allows black-box interop with Census. It does not
(yet) interop with census in the same process as Brave. However, this
should be a helpful step to those especially currently using Brave, but
trying out Census for non-java services.

See #626

@codefromthecrypt
Copy link
Member Author

I need to add an interop test to prove this actually works cc @bogdandrutu

@codefromthecrypt codefromthecrypt force-pushed the write-census-format branch 3 times, most recently from e4b4b58 to 7ef4a57 Compare April 11, 2018 12:11
@codefromthecrypt
Copy link
Member Author

note this will be refactored to be optional behavior

if (current != null) current.close();
}

@Test public void readsPropagatedSpan() throws Exception {
Copy link
Member Author

Choose a reason for hiding this comment

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

here's the test that shows we are participating in the same trace

@codefromthecrypt codefromthecrypt force-pushed the write-census-format branch 2 times, most recently from 4d211ef to 0712a35 Compare April 12, 2018 06:36
@codefromthecrypt
Copy link
Member Author

added censusPropagationEnabled property and client+server tests

@codefromthecrypt
Copy link
Member Author

going to add tags propagation as well, as we can lose it otherwise..

@codefromthecrypt
Copy link
Member Author

refactored to not drop tag context

@codefromthecrypt
Copy link
Member Author

not-dropping is probably not good enough, as the last upstream part of the tag context should be replaced..

@codefromthecrypt
Copy link
Member Author

ok rewrote everything. tags now works

@codefromthecrypt codefromthecrypt changed the title Reads and writes OpenCensus binary formatted trace header in gRPC Reads and writes gRPC trace and tags binary metadata Apr 30, 2018
spanId = readLong(bytes, pos);
pos += 8;
} else {
logger.fine("Invalid input: expected span ID at offset " + pos);
Copy link
Member Author

Choose a reason for hiding this comment

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

self-note: kill all these string cats as they shouldn't happen unless fine is enabled

@codefromthecrypt codefromthecrypt changed the base branch from refactor-extrac to master April 30, 2018 15:41
@codefromthecrypt codefromthecrypt force-pushed the write-census-format branch 4 times, most recently from 4e24b36 to bb000e1 Compare June 13, 2018 13:38

To enable this feature, set `grpcPropagationFormatEnabled` which is off
by default:
```java
Copy link
Member Author

Choose a reason for hiding this comment

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

To give most flexibility I made this a feature flag similar to how tracing is still a flag in gRPC

@codefromthecrypt
Copy link
Member Author

going to release this tomorrow unless screams to the contrary

This prefers the OpenCensus binary formatted trace header, falling back
to Brave defaults if absent, upon server requests. On the client side,
both are written.

This allows interop with gRPC services that don't use B3 (for example,
the default census module). This also acknowledges that the OpenCensus
binary format is fairly contained within gRPC, so doesn't need to become
a top-level propagation format, for example used with non-gRPC services
as yet.

Redundantly encoding this format has some cost to it, notable 14
character header name "grpc-trace-bin" and the 38 character base64
encoded binary context value. This also writes "grpc-tags-bin".

Note that this only allows black-box interop with Census. It does not
(yet) interop with census in the same process as Brave. However, this
should be a helpful step to those especially currently using Brave, but
trying out Census for non-java services.
@codefromthecrypt codefromthecrypt merged commit e13017a into master Jun 14, 2018
@codefromthecrypt codefromthecrypt deleted the write-census-format branch June 14, 2018 00:11
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant