Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Commit

Permalink
config: do not change state based off config results
Browse files Browse the repository at this point in the history
Since the OpenCensus interceptor config change hasn't
yet been implemented, we shouldn't be changing the exporters'
connection state based on its results -- it'll always return
disconnected since right now it is a noop/fail-fast on the agent.

I found this by a live integration test with a high traffic app.
  • Loading branch information
odeke-em committed Oct 16, 2018
1 parent ea77d4d commit 7528586
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 10 deletions.
2 changes: 1 addition & 1 deletion connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (ae *Exporter) connected() bool {
return atomic.LoadInt32(&ae.connectionState) == sConnected
}

const defaultConnReattemptPeriod = 800 * time.Millisecond
const defaultConnReattemptPeriod = 5000 * time.Millisecond

func (ae *Exporter) indefiniteBackgroundConnection() error {
defer func() {
Expand Down
23 changes: 14 additions & 9 deletions ocagent.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,11 +196,12 @@ func (ae *Exporter) dialToAgent() (*grpc.ClientConn, error) {
}

func (ae *Exporter) handleConfigStreaming(configStream agenttracepb.TraceService_ConfigClient) error {
// Note: We haven't yet implemented configuration sending so we
// should NOT be changing connection states within this function for now.
for {
recv, err := configStream.Recv()
if err != nil {
// TODO: Check if this is a transient error or exponential backoff-able.
ae.setStateDisconnected()
return err
}
cfg := recv.Config
Expand All @@ -224,7 +225,6 @@ func (ae *Exporter) handleConfigStreaming(configStream agenttracepb.TraceService
// Then finally send back to upstream the newly applied configuration
err = configStream.Send(&agenttracepb.CurrentLibraryConfig{Config: &tracepb.TraceConfig{Sampler: cfg.Sampler}})
if err != nil {
ae.setStateDisconnected()
return err
}
}
Expand Down Expand Up @@ -298,14 +298,19 @@ func (ae *Exporter) uploadTraces(sdl []*trace.SpanData) {
return

default:
if !ae.connected() {
return
}

protoSpans := ocSpanDataToPbSpans(sdl)
if len(protoSpans) > 0 && ae.connected() {
err := ae.traceExporter.Send(&agenttracepb.ExportTraceServiceRequest{
Spans: protoSpans,
})
if err != nil {
ae.setStateDisconnected()
}
if len(protoSpans) == 0 {
return
}
err := ae.traceExporter.Send(&agenttracepb.ExportTraceServiceRequest{
Spans: protoSpans,
})
if err != nil {
ae.setStateDisconnected()
}
}
}
Expand Down

0 comments on commit 7528586

Please sign in to comment.