unified logging pattern & facilities
Nobody has claimed this yet.
Assessment
- Difficulty
- 5/5
- Estimated time
- Over a week
- Newbie friendliness
- 25/100
- Issue type
- Refactor
- Clarity
- Mostly clear
- Activity status
- Stale
- Tech stack
- go
- Domain
- observability-sre
Research direction
Start by inspecting the salt/log package and its consumers to map the current Logrus/Zap abstraction, then check the mux package for logging-related middleware. Done means the formatted-logger abstraction is removed, Salt and ODPF usage assumes Zap structured logging, and any retained context helpers are explicitly justified.
Written by the indexing model from the issue text.
Description
Currently salt/log package tries to abstract Logrus and Zap into a common formatted-logger interface (i.e., Infof(msg string, args ...any), etc. ). While logrus is designed as a formatted-logger, uber/zap is specifically designed for efficient structured logging and this kind of abstraction nullifies the major benefit of it.
I propose we remove this abstraction altogether[^1] and assume direct usage of zap within salt and in ODPF applications that use salt. Benefits of doing this:
- All the benefits of structured logging (easy to parse logs, easy to search/filter by field values, easy to attach request context with each log, etc.)
- Not giving an abstracted formatted-logger will force us to always stick to structured logging.
- Assuming zap as the logger of choice allows us to provide certain useful utility abstractions. Few examples:
* A request-logging middleware inmuxpackage that automatically logs request info (method, path, client-ip, etc.) and response info (status, response time, etc.)
* A middleware for injecting request related context (req-id, current user id, the route info, etc.) intoreq.Context()so that every log in all the subsequent layers automatically add this to every log entry.
[^1]: We can still have some utility functions if we need to (e.g., a helper to inject log context into ctx). But attempting to abstract over logging functionality will not have justifiable benefits.
- Dominant language
- Go
- Stars
- 14
- Forks
- 8
- PR merge metrics
- No merged PRs in 30d
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from raystack/salt
-
Difficulty 5/5 Over a week Newbie friendliness 30/100
-
Difficulty 5/5 Over a week Newbie friendliness 35/100
-
Difficulty 4/5 3-5 days Newbie friendliness 42/100
-
Difficulty 4/5 3-5 days Newbie friendliness 25/100
-
enhancement
Difficulty 5/5 Over a week Newbie friendliness 25/100
Similar issues
-
optimization optimization:agents-md-curator
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
githubnext/gh-aw-cao#13143 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
blinklabs-io/bursa#904 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
yanet-platform/ipfw-go#129 ·
-
bug confmap/provider/googlesecretmanagerprovider needs triage
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
open-telemetry/opentelemetry-collector-contrib#51273 · 2 comments ·
-
bug: AI Gateway client filter lists "Unknown" twice when NULL and literal Unknown clients coexist Openbug
Difficulty 2/5 1-3 hours Newbie friendliness 90/100