Skip to content

Commit

Permalink
fix: standardize log field names to use underscores (#156)
Browse files Browse the repository at this point in the history
### TL;DR
Updated log field names to use underscores instead of hyphens for better compatibility with open telemetry fields.

### What changed?
Standardized all logging field names across the codebase to use underscore notation instead of hyphenated notation. For example:
- `narinfo-hash` → `narinfo_hash`
- `server-addr` → `server_addr`
- `nar-url` → `nar_url`
- `trace-id` → `trace_id`

### How to test?
1. Run the application and verify logs are being generated
2. Check that all log fields use underscore notation
3. Verify log parsers can properly process the updated field names
4. Ensure no logging functionality is broken

### Why make this change?
Using underscores in log field names is a common convention that improves compatibility with log parsing tools and systems. Many log processors handle underscore-separated fields better than hyphenated ones, making it easier to query and analyze logs. Furthermore, open-telemetry internally changes dashes to underscores (at least from what I observed in traces) and this makes it consistent.
  • Loading branch information
kalbasit authored Dec 25, 2024
1 parent 479750b commit c2cd2cd
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 36 deletions.
4 changes: 2 additions & 2 deletions cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ func New() *cli.Command {

(zerolog.Ctx(ctx)).
Info().
Str("otel-grpc-url", colURL).
Str("log-level", lvl.String()).
Str("otel_grpc_url", colURL).
Str("log_level", lvl.String()).
Msg("logger created")

return ctx, nil
Expand Down
4 changes: 2 additions & 2 deletions cmd/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func serveAction() cli.ActionFunc {
}

logger.Info().
Str("server-addr", cmd.String("server-addr")).
Str("server_addr", cmd.String("server-addr")).
Msg("Server started")

if err := server.ListenAndServe(); err != nil {
Expand Down Expand Up @@ -264,7 +264,7 @@ func createCache(

zerolog.Ctx(ctx).
Info().
Str("time-zone", loc.String()).
Str("time_zone", loc.String()).
Msg("setting up the cache timezone location")

c.SetupCron(ctx, loc)
Expand Down
34 changes: 17 additions & 17 deletions pkg/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ func (c *Cache) GetNarInfo(ctx context.Context, hash string) (*narinfo.NarInfo,

ctx = zerolog.Ctx(ctx).
With().
Str("narinfo-hash", hash).
Str("narinfo_hash", hash).
Logger().
WithContext(ctx)

Expand Down Expand Up @@ -576,7 +576,7 @@ func (c *Cache) pullNarInfo(
zerolog.Ctx(ctx).
Error().
Err(err).
Str("nar-url", narInfo.URL).
Str("nar_url", narInfo.URL).
Msg("error parsing the nar URL")

done()
Expand All @@ -592,8 +592,8 @@ func (c *Cache) pullNarInfo(

ctx = zerolog.Ctx(ctx).
With().
Str("nar-url", narInfo.URL).
Bool("zstd-support", enableZSTD).
Str("nar_url", narInfo.URL).
Bool("zstd_support", enableZSTD).
Logger().
WithContext(ctx)

Expand Down Expand Up @@ -665,7 +665,7 @@ func (c *Cache) PutNarInfo(ctx context.Context, hash string, r io.ReadCloser) er

ctx = zerolog.Ctx(ctx).
With().
Str("narinfo-hash", hash).
Str("narinfo_hash", hash).
Logger().
WithContext(ctx)

Expand Down Expand Up @@ -709,7 +709,7 @@ func (c *Cache) DeleteNarInfo(ctx context.Context, hash string) error {

ctx = zerolog.Ctx(ctx).
With().
Str("narinfo-hash", hash).
Str("narinfo_hash", hash).
Logger().
WithContext(ctx)

Expand Down Expand Up @@ -828,7 +828,7 @@ func (c *Cache) getNarInfoFromStore(ctx context.Context, hash string) (*narinfo.
zerolog.Ctx(ctx).
Error().
Err(err).
Str("nar-url", ni.URL).
Str("nar_url", ni.URL).
Msg("error parsing the nar-url")

// narinfo is invalid, remove it
Expand Down Expand Up @@ -1153,7 +1153,7 @@ func (c *Cache) runLRU(ctx context.Context) func() {

log := zerolog.Ctx(ctx).With().
Str("op", "lru").
Uint64("max-size", c.maxSize).
Uint64("max_size", c.maxSize).
Logger()

log.Info().Msg("running LRU")
Expand Down Expand Up @@ -1186,7 +1186,7 @@ func (c *Cache) runLRU(ctx context.Context) func() {
return
}

log = log.With().Float64("nar-total-size", narTotalSize.Float64).Logger()
log = log.With().Float64("nar_total_size", narTotalSize.Float64).Logger()

if uint64(narTotalSize.Float64) <= c.maxSize {
log.Info().Msg("store size is less than max-size, not removing any nars")
Expand All @@ -1196,7 +1196,7 @@ func (c *Cache) runLRU(ctx context.Context) func() {

cleanupSize := uint64(narTotalSize.Float64) - c.maxSize

log = log.With().Uint64("cleanup-size", cleanupSize).Logger()
log = log.With().Uint64("cleanup_size", cleanupSize).Logger()

log.Info().Msg("going to remove nars")

Expand All @@ -1213,20 +1213,20 @@ func (c *Cache) runLRU(ctx context.Context) func() {
return
}

log.Info().Int("count-nars", len(nars)).Msg("found this many nars to remove")
log.Info().Int("count_nars", len(nars)).Msg("found this many nars to remove")

narInfoHashesToRemove := make([]string, 0, len(nars))
narURLsToRemove := make([]nar.URL, 0, len(nars))

for _, narRecord := range nars {
narInfo, err := c.db.WithTx(tx).GetNarInfoByID(ctx, narRecord.NarInfoID)
if err == nil {
log.Info().Str("narinfo-hash", narInfo.Hash).Msg("deleting narinfo record")
log.Info().Str("narinfo_hash", narInfo.Hash).Msg("deleting narinfo record")

if _, err := c.db.WithTx(tx).DeleteNarInfoByHash(ctx, narInfo.Hash); err != nil {
log.Error().
Err(err).
Str("narinfo-hash", narInfo.Hash).
Str("narinfo_hash", narInfo.Hash).
Msg("error removing narinfo from database")
}

Expand All @@ -1238,12 +1238,12 @@ func (c *Cache) runLRU(ctx context.Context) func() {
Msg("error fetching narinfo from the database")
}

log.Info().Str("nar-hash", narRecord.Hash).Msg("deleting nar record")
log.Info().Str("nar_hash", narRecord.Hash).Msg("deleting nar record")

if _, err := c.db.WithTx(tx).DeleteNarByHash(ctx, narRecord.Hash); err != nil {
log.Error().
Err(err).
Str("nar-hash", narRecord.Hash).
Str("nar_hash", narRecord.Hash).
Msg("error removing nar from database")
}

Expand All @@ -1265,7 +1265,7 @@ func (c *Cache) runLRU(ctx context.Context) func() {
go func() {
defer wg.Done()

log := log.With().Str("narinfo-hash", hash).Logger()
log := log.With().Str("narinfo_hash", hash).Logger()

log.Info().Msg("deleting narinfo from store")

Expand All @@ -1283,7 +1283,7 @@ func (c *Cache) runLRU(ctx context.Context) func() {
go func() {
defer wg.Done()

log := log.With().Str("nar-url", narURL.String()).Logger()
log := log.With().Str("nar_url", narURL.String()).Logger()

log.Info().Msg("deleting nar from store")

Expand Down
12 changes: 6 additions & 6 deletions pkg/cache/upstream/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func New(ctx context.Context, u *url.URL, pubKeys []string) (Cache, error) {

ctx = zerolog.Ctx(ctx).
With().
Str("upstream-url", c.url.String()).
Str("upstream_url", c.url.String()).
Logger().
WithContext(ctx)

Expand Down Expand Up @@ -118,9 +118,9 @@ func (c Cache) GetNarInfo(ctx context.Context, hash string) (*narinfo.NarInfo, e

ctx = zerolog.Ctx(ctx).
With().
Str("narinfo-hash", hash).
Str("narinfo-url", u).
Str("upstream-url", c.url.String()).
Str("narinfo_hash", hash).
Str("narinfo_url", u).
Str("upstream_url", c.url.String()).
Logger().
WithContext(ctx)

Expand Down Expand Up @@ -194,8 +194,8 @@ func (c Cache) GetNar(ctx context.Context, narURL nar.URL, mutators ...func(*htt
ctx = narURL.NewLogger(
zerolog.Ctx(ctx).
With().
Str("nar-url", u).
Str("upstream-url", c.url.String()).
Str("nar_url", u).
Str("upstream_url", c.url.String()).
Logger(),
).WithContext(ctx)

Expand Down
6 changes: 3 additions & 3 deletions pkg/nar/url.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ func ParseURL(u string) (URL, error) {
// NewLogger returns a new logger with the right fields.
func (u URL) NewLogger(log zerolog.Logger) zerolog.Logger {
return log.With().
Str("nar-hash", u.Hash).
Str("nar-compression", u.Compression.String()).
Str("nar-query", u.Query.Encode()).
Str("nar_hash", u.Hash).
Str("nar_compression", u.Compression.String()).
Str("nar_query", u.Query.Encode()).
Logger()
}

Expand Down
12 changes: 6 additions & 6 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,21 +120,21 @@ func requestLogger(next http.Handler) http.Handler {

log := zerolog.Ctx(r.Context()).With().
Str("method", r.Method).
Str("request-uri", r.RequestURI).
Str("request_uri", r.RequestURI).
Str("from", r.RemoteAddr).
Logger()

if span.SpanContext().HasTraceID() {
log = log.
With().
Str("trace-id", span.SpanContext().TraceID().String()).
Str("trace_id", span.SpanContext().TraceID().String()).
Logger()
}

if span.SpanContext().HasSpanID() {
log = log.
With().
Str("span-id", span.SpanContext().SpanID().String()).
Str("span_id", span.SpanContext().SpanID().String()).
Logger()
}

Expand Down Expand Up @@ -220,7 +220,7 @@ func (s *Server) getNarInfo(withBody bool) http.HandlerFunc {
r = r.WithContext(
zerolog.Ctx(ctx).
With().
Str("narinfo-hash", hash).
Str("narinfo_hash", hash).
Logger().
WithContext(ctx))

Expand Down Expand Up @@ -281,7 +281,7 @@ func (s *Server) putNarInfo(w http.ResponseWriter, r *http.Request) {
r = r.WithContext(
zerolog.Ctx(ctx).
With().
Str("narinfo-hash", hash).
Str("narinfo_hash", hash).
Logger().
WithContext(ctx))

Expand Down Expand Up @@ -321,7 +321,7 @@ func (s *Server) deleteNarInfo(w http.ResponseWriter, r *http.Request) {
r = r.WithContext(
zerolog.Ctx(ctx).
With().
Str("narinfo-hash", hash).
Str("narinfo_hash", hash).
Logger().
WithContext(ctx))

Expand Down

0 comments on commit c2cd2cd

Please sign in to comment.