-
Notifications
You must be signed in to change notification settings - Fork 1.3k
All Sourcegraph components can export uniform, structured logs #33192
Comments
Brainstorming and feedback discussion: https://github.com/sourcegraph/sourcegraph/discussions/33248 |
This introduces a new logging library we plan to enforce as the standard for logging at Sourcegraph - see https://github.com/sourcegraph/sourcegraph/issues/33192 for more details. Co-authored-by: Eric Fritz <eric@eric-fritz.com>
FYI I have been looking into OpenTelemetry as we can potentially leverage an existing standard for log data. The summary of my findings is available in this document. From my findings, I think that our work in the logging improvements seems well-aligned with OpenTelemetry’s log models, and we’ve been mindful about committing to this format early on. I recommend we proceed as planned. |
Progress: We have a new logging library that encodes some best practices:
How to add logging and Add observability have been added to the docs. We are working on migrating logs to this new library. See The next step is to investigate which migrations we can delegate to Gitstart. |
Note that the code for this has moved to https://github.com/sourcegraph/log, as of https://github.com/sourcegraph/sourcegraph/pull/36834. There are currently no plans to move the docs, etc and we will probably continue to refer to it as |
I am marking this as DONE - the initial goals have been achieved, the API and featureset has stabilized, and the remaining work to bring migration to 100% and address data redaction will be tracked separately in https://github.com/sourcegraph/sourcegraph/issues/37586 |
Problem to solve
Today,
log15
is used throughout the Sourcegraph codebase for logging. It is mostly sufficient, but has some issues:dbug
,eror
, etc. which is a common source of confusion, and causes interop issues with external toolslog15
's commit activity is from over 5 years ago, and the repository has a number of abandoned open PRs. We are unlikely to run intolog4j
-style security risk, but this is still a potential risk.log15
(reference) - at Sourcegraph's logging volume this is a likely a non-negligible differenceWe also have issues such as extensive use of global logging, which does not include any structured metadata, and extensive use of unstructured logs in general. This leads to a low signal to noise ratio, and a lack of ability to filter logs in a useful manner.
We can take this opportunity to improve tie-in with OpenTracing standards, and improve our sensitive data redaction in logging.
Measure of success
now https://github.com/sourcegraph/log)lib/log
Solution summary
We will propose a complete migration to zap (which would work for Cloud as well as on-prem) and encourage use of passing loggers instances around to enable better structured logging by attaching metadata to logger instances and log entries that can be propagated. Part of this will likely include banning the use of other printing/logging packages other than the chosen logging library.
Artifacts:
What specific customers are we iterating on the problem and solution with?
Internal Sourcegraph developers.
Impact on use cases
This effort contributes to the company-wide effort to improve Observability.
Delivery plan
Tracked issues
@unassigned
Completed
@bobheadxi: 8.00d
Completed: 8.00d
@burmudar: 6.00d
Completed: 5.00d
@davejrt: 3.00d
Completed: 3.00d
@gitstart-sourcegraph: 8.50d
Completed: 4.50d
#35516) 2.00d@jhchabran
Completed
@marekweb: 1.00d
Completed: 1.00d
@varungandhi-src
Legend
The text was updated successfully, but these errors were encountered: