Skip to content

Implement timestamped logs

Arran Walker requested to merge ajwalker/timestampable-logs into main

What does this MR do?

Several commits:

  • Completely detach masking from the trace buffer. Removing SetMasked and thus all masking (temporarily, we add it back)
  • Make improved URL matching the default (and remove the FF this was behind)
  • Refactored SSH Client because it needlessly made attaching a new log stream to it difficult
  • Refactored logging so that each log stream has its own secret maskers (welcome back maskers)
  • Add timestamps functionality behind feature flag

The log format is described in timestamper.go and attempts to strike a balance between human and machine readability, with fixed sized buffers to be performant to write and to parse.

Why was this MR needed?

Adding timestamps is hard to achieve because:

  • There's multiple streams writing to the trace
  • Adding timestamps involves breaking up the streams by new lines and when there's very large log lines
  • Multiple streams and the breaking up an incoming stream can easily break the secret maskers.

This MR addresses these problems by moving secret maskers to each individual stream and then adds timestamps. This results in large changes throughout, because both masking and timestamp writers require a call to Close() in the correct locations and order so that data is flushed from the writers at the end of a stream.

What's the best way to test this MR?

  • Existing tests should pass
  • Existing jobs should run with existing functionality (masking etc.)
  • Timestamps will be added to logs when FF_TIMESTAMPS is enabled.

What are the relevant issue numbers?

Closes #36888 (closed) Closes #36887 (closed)

Related to gitlab#202293

Edited by Arran Walker

Merge request reports