A72: OpenTelemetry Tracing #389
Conversation
YifeiZhuang
force-pushed
the
ot-tracing
branch
from
26ab562
to
43d09ef
Compare
YifeiZhuang
force-pushed
the
ot-tracing
branch
from
43d09ef
to
0041641
Compare
The span name will inherit what census uses, like in server
But it looks A45 does not mention |
ejona's language suggestion Co-authored-by: Eric Anderson <ejona@google.com>
…into ot-tracing
1. [done] Add that OpenTelemetry module should document that things/configurations will change as OT changes. 2. [done] Move C++ fast path to rationale. Capture two alternatives about the fast path that may have in the future. 3. [done] Update C++ API about adding a new method in OpenTelemetryPluginBuilder, and reference to the current code and the gRFC. 4. [todo] complete the C++ GrpcTraceBinPropagator and GrpcTextMapCarrier
Fill up C++ sections
YifeiZhuang
force-pushed
the
ot-tracing
branch
from
81dd9a4
to
517f340
Compare
|
@markdroth thanks for the suggestions. They are great. Please take a look again. @dfawley kind reminder please review and approve. |
A72-open-telemetry-tracing.md
Outdated
| #### Go | ||
| In Go, the OpenTelemetry stream tracers and interceptors will be provided for users to install. | ||
|
|
||
| TODO: add Go API. |
@markdroth Thanks for the quick feedbacks, I should have resolved all the comments. Please take a look again. |
|
@ejona86 @yashykt @markdroth @zasweq @dfawley updated tracing info based on conversation yesterday. Please review. Thanks! |
| (it depends on implementation whether there is a single event or two separate | ||
| events for compressed/uncompressed message sizes, the same below) | ||
| with name "Outbound message sent" and the following attributes: | ||
| * key `message.event.type` with string value "SENT". |
There was a problem hiding this comment.
is this attribute "message.event.type" useful? The message already says sent?
| * When the application sends an outbound message to the transport, add Event(s) | ||
| (it depends on implementation whether there is a single event or two separate | ||
| events for compressed/uncompressed message sizes, the same below) | ||
| with name "Outbound message sent" and the following attributes: |
There was a problem hiding this comment.
nit: feel free to ignore
Is "outbound" adding any value here?
| events for compressed/uncompressed message sizes, the same below) | ||
| with name "Outbound message sent" and the following attributes: | ||
| * key `message.event.type` with string value "SENT". | ||
| * key `message.message.id` with integer value of the seq no. The seq no. indicates |
There was a problem hiding this comment.
how was the namespacing for "message.message.id" decided?
There was a problem hiding this comment.
It was from the Otel java opencensus shim. Languages can be different.
I think whenever possible, we can make the name generated from the shim and the one generated directly from otel be consistent. This is not happening because the naming in our census needs improvements. And I am fine with that. @markdroth @ejona86
…remove to transport and form wire
…ssed message size inbound
| tracer factory API will be stabilized. OpenTelemetryModule will be created with | ||
| an OpenTelemetry API instance passing in for necessary configurations. | ||
| Users can also rely on SDK autoconfig extension that configures the sdk object | ||
| through environmental variables or Java system properties, then pass the |
There was a problem hiding this comment.
Nit: "environment" variables, not "environmental".
| // DialOption returns a dial option which enables OpenTelemetry instrumentation | ||
| // code for a grpc.ClientConn. | ||
| // | ||
| // Client applications interested in instrumenting their grpc.ClientConn should | ||
| // pass the dial option returned from this function as a dial option to | ||
| // grpc.Dial(). | ||
| // | ||
| // Using this option will always lead to instrumentation, however in order to | ||
| // use the data a SpanExporter must be registered with the TraceProvider in the | ||
| // TraceOption. Client side has retries, so a Unary and Streaming Interceptor are | ||
| // registered to handle per RPC traces, and a Stats Handler is registered to handle | ||
| // per RPC attempt trace. These three components registered work together in | ||
| // conjunction, and do not work standalone. | ||
| func DialOption(to TraceOptions) grpc.DialOption {} | ||
|
|
||
| // ServerOption returns a server option which enables OpenTelemetry | ||
| // instrumentation code for a grpc.Server. | ||
| // | ||
| // Server applications interested in instrumenting their grpc.Server should pass | ||
| // the server option returned from this function as an argument to | ||
| // grpc.NewServer(). | ||
| // | ||
| // Using this option will always lead to instrumentation, however in order to | ||
| // use the data a SpanExporter must be registered with the TraceProvider option. | ||
| // Server side does not have retries, so a registered Stats Handler is the only | ||
| // option that is returned. | ||
| func ServerOption(to TraceOptions) grpc.ServerOption {} |
There was a problem hiding this comment.
These bits are pre-existing from after Metrics support is added. I think instead we'll just be adding a field called TraceOptions to opentelemetry.Options
There was a problem hiding this comment.
Yeah we will. Options will have Metrics Options as the in flight PR, I'll add tracing options, and eventually Logging Options.
There was a problem hiding this comment.
oh I see this in the open grpc/grpc-go#7166, This will be outdated but i think not necessary to keep all updated - we are racing with ourselves.
Design doc: go/grpc-open-telemetry-tracing-design
cc. @yashykt @zasweq @XuanWang-Amos