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.

Sign up for GitHub

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

Jump to bottom

fix: go-kit/log-slog adapter fixes #9943

Closed
wants to merge 16 commits into from
Closed

fix: go-kit/log-slog adapter fixes #9943

wants to merge 16 commits into from

Commits on Nov 19, 2024

  1. Upgrade to latest mimir-prometheus@main 

    Signed-off-by: Arve Knudsen <[email protected]>
    @aknuds1
    aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    d920e20 View commit details
    Browse the repository at this point in the history

  2. Sync test cases with upstream

    @charleskorn @aknuds1
    charleskorn authored and aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    1d49553 View commit details
    Browse the repository at this point in the history

  3. Adjust behaviour of range vector selectors and subqueries to match ne… 

    …w left-open behaviour in Prometheus 3
    @charleskorn @aknuds1
    charleskorn authored and aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    7f54618 View commit details
    Browse the repository at this point in the history

  4. Adjust TestRangeVectorSelectors and TestAnnotations to match new … 

    …left-open interval behaviour
    @charleskorn @aknuds1
    charleskorn authored and aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    a07db7d View commit details
    Browse the repository at this point in the history

  5. Disable unsupported upstream test cases

    @charleskorn @aknuds1
    charleskorn authored and aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    2bb9e7d View commit details
    Browse the repository at this point in the history

  6. Update our test cases to reflect changes in Prometheus' behaviour

    @charleskorn @aknuds1
    charleskorn authored and aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    a40f265 View commit details
    Browse the repository at this point in the history

  7. Ensure point slice lengths are the same before comparing them

    @charleskorn @aknuds1
    charleskorn authored and aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    3e96e18 View commit details
    Browse the repository at this point in the history

  8. Emit "incompatible sample types" annotations for vector/vector binary… 

    … operations
    @charleskorn @aknuds1
    charleskorn authored and aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    55be462 View commit details
    Browse the repository at this point in the history

  9. Emit "incompatible sample types" annotations for vector/scalar binary… 

    … operations
    @charleskorn @aknuds1
    charleskorn authored and aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    90570ac View commit details
    Browse the repository at this point in the history

  10. Modify vector/scalar binary comparison operations to match Prometheus… 

    …' new behaviour
    @charleskorn @aknuds1
    charleskorn authored and aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    66a522b View commit details
    Browse the repository at this point in the history

  11. Add extra test for subqueries

    @charleskorn @aknuds1
    charleskorn authored and aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    2450fe3 View commit details
    Browse the repository at this point in the history

  12. Bring in changes from prometheus/prometheus#15245

    @charleskorn @aknuds1
    charleskorn authored and aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    e0de5ba View commit details
    Browse the repository at this point in the history

  13. Update tests to match new binary operations behaviour for native hist… 

    …ograms
    @charleskorn @aknuds1
    charleskorn authored and aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    66da12a View commit details
    Browse the repository at this point in the history

  14. Bring in further changes from prometheus/prometheus#15413

    @charleskorn @aknuds1
    charleskorn authored and aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    e86fb40 View commit details
    Browse the repository at this point in the history

  15. Change behaviour to mirror prometheus/prometheus#15245 

    Signed-off-by: Arve Knudsen <[email protected]>
    @charleskorn @aknuds1
    charleskorn authored and aknuds1 committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    fe18ce2 View commit details
    Browse the repository at this point in the history

  16. fix: go-kit/log-slog adapter fixes 

    Changes included:
- log/slog always includes timestamps by default; they are now included
  in the go-kit/log output. We make no attempts to adjust/remove any
other timestamp keys (such as the often used `ts` kv pair in go-kit/log
implementations).
- a new function to handle properly appending/nesting our kv pairs for
  the logger
- proper handling of nested grouping
- introduction of a `preformatted` field on the goKitHandler struct to
  cache previously added kv pairs containing log values. helpful to
maintain the ordering of kv pairs added via multiple/nested/chained
calls to a logger's `.With()` or `.WithGroup()` methods.
- WithGroup() now returns a new handler struct, as opposed to updating
  itself (as recommended by slog implementation docs).
- Update test suite to allow stripping timestamps from logs to prevent
  non-deterministic output
- Update test suite mock setup to validate correct log formatting, now
  that kv pairs for logs follow the pattern `group1.group2.key=value`

Not addressed:
- I haven't found a good way to (programmatically) address call depth
  when getting source caller info. Using a call depth of 6 (see code
snippet) seems to properly unwind back to the originating `.Info()` etc
method call, but I haven't checked other options.

Example output, after shimming the following logging calls into the
existing handler test suite:

```go
gkadapterLogger := SlogFromGoKit(log.With(log.NewLogfmtLogger(os.Stderr), "caller", log.Caller(6)))
gkadapterLogger.Info("go-kit/log adapter output")
gkadapterLogger.WithGroup("g1").WithGroup("g2").Info("test1", "hello", "world")
gkadapterLogger.WithGroup("g1").With("hello", "world").Info("test2", "hello", "world")
gkadapterLogger.WithGroup("g1").With("hello", "world").WithGroup("g2").Info("test3", "hello", "world")
gkadapterLogger.With("hello", "world").With("hello", "world").Info("test4", "hello", "world")
// Output:
// level=info caller=slogadapter_test.go:26 time=2024-11-18T13:09:59.674350655-05:00 msg=test1 g1.g2.hello=world
// level=info caller=slogadapter_test.go:27 time=2024-11-18T13:09:59.674376383-05:00 msg=test2 g1.hello=world g1.hello=world
// level=info caller=slogadapter_test.go:28 time=2024-11-18T13:09:59.674392598-05:00 msg=test3 g1.hello=world g1.g2.hello=world
// level=info caller=slogadapter_test.go:29 time=2024-11-18T13:09:59.674419421-05:00 msg=test4 hello=world hello=world hello=world
```

Signed-off-by: TJ Hoplock <[email protected]>
    @tjhop
    tjhop committed Nov 19, 2024
    Configuration menu
    Copy the full SHA
    dc6f3da View commit details
    Browse the repository at this point in the history