Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

ddtrace/tracer: add interchangeable writer to logTraceWriter #939

Merged
merged 16 commits into from
Jul 5, 2021
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ddtrace/tracer/tracer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,12 @@ func TestTracerCleanStop(t *testing.T) {
}()
}

defer setLogWriter(ioutil.Discard)()
wg.Add(1)
go func() {
defer wg.Done()
for i := 0; i < n; i++ {
// Lambda mode is used to avoid the startup cost associated with agent discovery.
Start(withTransport(transport), WithLambdaMode(true))
time.Sleep(time.Millisecond)
Start(withTransport(transport), WithLambdaMode(true), WithSampler(NewRateSampler(0.99)))
Expand Down
13 changes: 12 additions & 1 deletion ddtrace/tracer/writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,17 @@ func (h *agentTraceWriter) flush() {
h.payload = newPayload()
}

// logWriter specifies the output target of the logTraceWriter; replaced in tests.
var logWriter io.Writer = os.Stdout
knusbaum marked this conversation as resolved.
Show resolved Hide resolved

// setLogWriter sets the io.Writer that any new logTraceWriter will write to and returns a function
// which will return the io.Writer to its original value.
func setLogWriter(w io.Writer) func() {
Copy link
Contributor

@gbbr gbbr Jun 22, 2021

Choose a reason for hiding this comment

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

I think this func is only needed in tests right? If so, does it make sense to move it into a test file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered it, but since var logWriter is declared and used here, I thought the function for manipulating it should be co-located so the intention/usage is apparent for whoever next looks at var logWriter.

Let me know if you feel strongly that it should be moved.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's test code (unused elsewhere), so it belongs in a test. But likewise, if you feel strongly, leave it :)

tmp := logWriter
logWriter = w
return func() { logWriter = tmp }
}

// logTraceWriter encodes traces into a format understood by the Datadog Forwarder
// (https://github.com/DataDog/datadog-serverless-functions/tree/master/aws/logs_monitoring)
// and writes them to os.Stdout. This is used to send traces from an AWS Lambda environment.
Expand All @@ -117,7 +128,7 @@ type logTraceWriter struct {
func newLogTraceWriter(c *config) *logTraceWriter {
w := &logTraceWriter{
config: c,
w: os.Stdout,
w: logWriter,
}
w.resetBuffer()
return w
Expand Down