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

Instrument interceptors with OpenCensus #63

Closed
bogdandrutu opened this issue Oct 4, 2018 · 8 comments
Closed

Instrument interceptors with OpenCensus #63

bogdandrutu opened this issue Oct 4, 2018 · 8 comments
Labels
enhancement New feature or request

Comments

@bogdandrutu
Copy link

Trace

For trace, we start a Span every time we start a new batch (will end when we call the ReceiveSpans). Because the RPC is a bidirectional and can live long we need to decouple this span from the RPC span, but because they are related we should add the RPC span as parent link (here is a good example where links as useful).

Stats

Measure:

  • oc.io/interceptor/received_spans - long value

Tags:

  • opencensus_interceptor

Measurement:
For oc.io/interceptor/received_spans we record a measurement every time we receive a new message from the client even if the number is 0.

Views:
We should install a distribution view with buckets {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12, 14, 16, 18, 20, 25, 30, 35, 40, 45, 50, 60, 70, 80, 90, 100, 150, 200, 250, 300, 450, 500, 600, 700, 800, 900, 1000, 1200, 1400, 1600, 1800, 2000}

@songy23 songy23 added enhancement New feature or request P2 labels Oct 4, 2018
odeke-em added a commit that referenced this issue Oct 5, 2018
Instrument interceptor/opencensus with stats to record
the number of received spans. This is accomplished by
a helper to process spans that sends over spans even
if the number of spans is 0. Also record with tag_key
"opencensus_interceptor" whose value is the name
of the respective interceptor.

The test to ensure that 0 length spans are also added
is currently disabled because:
* Issue census-instrumentation/opencensus-go#862
is not yet implemented which requests that the OpenCensus-Go
stats worker provide a method Flush to flush all data. Without
it, metrics will contain stale data from previous tests.

Add tests to lock-in this behavior.

Updates #63
odeke-em added a commit that referenced this issue Oct 5, 2018
Instrument interceptor/opencensus with stats to record
the number of received spans. This is accomplished by
a helper to process spans that sends over spans even
if the number of spans is 0. Also record with tag_key
"opencensus_interceptor" whose value is the name
of the respective interceptor.

The test to ensure that 0 length spans are also added
is currently disabled because:
* Issue census-instrumentation/opencensus-go#862
is not yet implemented which requests that the OpenCensus-Go
stats worker provide a method Flush to flush all data. Without
it, metrics will contain stale data from previous tests.

Add tests to lock-in this behavior.

Updates #63
odeke-em added a commit that referenced this issue Oct 5, 2018
Instrument interceptor/opencensus with stats to record
the number of received spans. This is accomplished by
a helper to process spans that sends over spans even
if the number of spans is 0. Also record with tag_key
"opencensus_interceptor" whose value is the name
of the respective interceptor.

The test to ensure that 0 length spans are also added
is currently disabled because:
* Issue census-instrumentation/opencensus-go#862
is not yet implemented which requests that the OpenCensus-Go
stats worker provide a method Flush to flush all data. Without
it, metrics will contain stale data from previous tests.

Add tests to lock-in this behavior.

Updates #63
@bogdandrutu
Copy link
Author

Please also install the ocgrpc plugin for the interceptor gRPC server.

@odeke-em
Copy link
Member

odeke-em commented Oct 5, 2018

Great suggestions @bogdandrutu! Also I think in the future(after we've finished rolling out this instrumentation), would it make sense for us to extract information from the node? For example:

  • process_identifier --> "opencensus_task" for stats and perhaps service name for metrics?

@odeke-em
Copy link
Member

odeke-em commented Oct 6, 2018

Same thoughts, I actually have a TODO in the code as per

// TODO: (@odeke-em) in the future, also add OpenCensus
// stats handlers to start this gPRC server.
srv := grpc.NewServer()

but I think the even better way would be to create a helper method say in internal that creates a gRPC server for us since every interceptor that needs it will have to remember to install the ocgrpc plugin which could become error-prone.

odeke-em added a commit that referenced this issue Oct 6, 2018
Instrument interceptor/opencensus with stats to record
the number of received spans. This is accomplished by
a helper to process spans that sends over spans even
if the number of spans is 0. Also record with tag_key
"opencensus_interceptor" whose value is the name
of the respective interceptor.

The test to ensure that 0 length spans are also added
is currently disabled because:
* Issue census-instrumentation/opencensus-go#862
is not yet implemented which requests that the OpenCensus-Go
stats worker provide a method Flush to flush all data. Without
it, metrics will contain stale data from previous tests.

Add tests to lock-in this behavior.

Updates #63
odeke-em added a commit that referenced this issue Oct 7, 2018
Instrument interceptor/opencensus with stats to record
the number of received spans. This is accomplished by
a helper to process spans that sends over spans even
if the number of spans is 0. Also record with tag_key
"opencensus_interceptor" whose value is the name
of the respective interceptor.

The test to ensure that 0 length spans are also added
is currently disabled because:
* Issue census-instrumentation/opencensus-go#862
is not yet implemented which requests that the OpenCensus-Go
stats worker provide a method Flush to flush all data. Without
it, metrics will contain stale data from previous tests.

Add tests to lock-in this behavior.

Updates #63
@bogdandrutu
Copy link
Author

@odeke-em just having a todo to also add tags from the Node is fine for the moment.

odeke-em added a commit that referenced this issue Oct 7, 2018
Instrument interceptor/opencensus with stats to record
the number of received spans. This is accomplished by
a helper to process spans that sends over spans even
if the number of spans is 0. Also record with tag_key
"opencensus_interceptor" whose value is the name
of the respective interceptor.

The test to ensure that 0 length spans are also added
is currently disabled because:
* Issue census-instrumentation/opencensus-go#862
is not yet implemented which requests that the OpenCensus-Go
stats worker provide a method Flush to flush all data. Without
it, metrics will contain stale data from previous tests.

Add tests to lock-in this behavior.

Also replace grpc.NewServer with a stats enabled server helper call.
This enables tracing and metrics for each gRPC server by replacing
naked usages of grpc.NewServer with the new helper function:
    internal.GRPCServerWithObservabilityEnabled

Updates #63
@odeke-em
Copy link
Member

odeke-em commented Oct 8, 2018

@odeke-em just having a todo to also add tags from the Node is fine for the moment.

@bogdandrutu perfect. I've done that!

Because the RPC is a bidirectional and can live long we need to decouple this span from the RPC span, but because they are related we should add the RPC span as parent link (here is a good example where links as useful).

EDIT: I'll take a look at this right now.

odeke-em added a commit that referenced this issue Oct 8, 2018
Trace instrument the section that performs batch uploading
to the spanreceiver. The created span has a parent link
from the initiating RPC's span. Added tests to ensure that
these conditions always hold.

Updates #63
odeke-em added a commit that referenced this issue Oct 9, 2018
Trace instrument the section that performs batch uploading
to the spanreceiver. The created span has a parent link
from the initiating RPC's span. Added tests to ensure that
these conditions always hold.

Updates #63
odeke-em added a commit that referenced this issue Oct 9, 2018
Trace instrument the section that performs batch uploading
to the spanreceiver. The created span has a parent link
from the initiating RPC's span. Added tests to ensure that
these conditions always hold.

Updates #63
odeke-em added a commit that referenced this issue Oct 9, 2018
Trace instrument the section that performs batch uploading
to the spanreceiver. The created span has a parent link
from the initiating RPC's span. Added tests to ensure that
these conditions always hold.

Updates #63
odeke-em added a commit that referenced this issue Oct 9, 2018
Trace instrument the section that performs batch uploading
to the spanreceiver. The created span has a parent link
from the initiating RPC's span. Added tests to ensure that
these conditions always hold.

Updates #63
@odeke-em
Copy link
Member

odeke-em commented Oct 9, 2018

Alright, so with PRs:

PR Functionality
#65 Metrics for the interceptor
#76 Tracing the OpenCensus interceptor

I believe this issue is solved. However, we'll need to keep this information and ideas in when we implement other interceptors. PTAL and feel free to reopen.

@odeke-em odeke-em closed this as completed Oct 9, 2018
@bogdandrutu
Copy link
Author

Maybe add a md file in the interceptor about how to instrument any interceptor.

@odeke-em
Copy link
Member

odeke-em commented Oct 9, 2018

In deed, great suggestion @bogdandrutu, I filed #79.

fivesheep pushed a commit to fivesheep/opencensus-service that referenced this issue Jun 12, 2019
Instrument interceptor/opencensus with stats to record
the number of received spans. This is accomplished by
a helper to process spans that sends over spans even
if the number of spans is 0. Also record with tag_key
"opencensus_interceptor" whose value is the name
of the respective interceptor.

The test to ensure that 0 length spans are also added
is currently disabled because:
* Issue census-instrumentation/opencensus-go#862
is not yet implemented which requests that the OpenCensus-Go
stats worker provide a method Flush to flush all data. Without
it, metrics will contain stale data from previous tests.

Add tests to lock-in this behavior.

Also replace grpc.NewServer with a stats enabled server helper call.
This enables tracing and metrics for each gRPC server by replacing
naked usages of grpc.NewServer with the new helper function:
    internal.GRPCServerWithObservabilityEnabled

Updates census-instrumentation#63
fivesheep pushed a commit to fivesheep/opencensus-service that referenced this issue Jun 12, 2019
Trace instrument the section that performs batch uploading
to the spanreceiver. The created span has a parent link
from the initiating RPC's span. Added tests to ensure that
these conditions always hold.

Updates census-instrumentation#63
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants