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

spanlogger: fix double logging of caller information #604

Merged
merged 9 commits into from
Oct 13, 2024
Merged

spanlogger: fix double logging of caller information #604

merged 9 commits into from
Oct 13, 2024
+128 −54

Commits on Oct 9, 2024

  1. Extend existing tests to catch issue

    @charleskorn
    charleskorn committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    2d8e068 View commit details
    Browse the repository at this point in the history

  2. Add further test cases

    @charleskorn
    charleskorn committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    d8ed57a View commit details
    Browse the repository at this point in the history

  3. Fix issue where caller information is logged twice

    @charleskorn
    charleskorn committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    e6d3111 View commit details
    Browse the repository at this point in the history

  4. Add benchmark comparing the performance of Caller and `SpanLoggerAw… 

    …areCaller`
    @charleskorn
    charleskorn committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    e93c3a0 View commit details
    Browse the repository at this point in the history

  5. Change name to satisfy linter

    @charleskorn
    charleskorn committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    2c3d911 View commit details
    Browse the repository at this point in the history

  6. Fix tests broken by removal of caller key/value pair from SpanLogger

    @charleskorn
    charleskorn committed Oct 9, 2024
    Configuration menu
    Copy the full SHA
    647f153 View commit details
    Browse the repository at this point in the history

Commits on Oct 10, 2024

  1. Don't bother with checking for go-kit/log stack frames until we've se… 

    …en SpanLogger

This makes the cost of spanlogger.Caller negligible for use outside of
SpanLogger:

                            │ SpanLoggerAwareCaller/with_go-kit's_Caller-10 │ SpanLoggerAwareCaller/with_dskit's_spanlogger.Caller-10 │
                            │                    sec/op                     │                 sec/op                  vs base         │
comparison-after-step-2.txt                                     927.5n ± 5%                              932.1n ± 2%  ~ (p=0.093 n=6)

                            │ SpanLoggerAwareCaller/with_go-kit's_Caller-10 │ SpanLoggerAwareCaller/with_dskit's_spanlogger.Caller-10 │
                            │                     B/op                      │                  B/op                   vs base         │
comparison-after-step-2.txt                                      732.5 ± 0%                               733.0 ± 0%  ~ (p=0.636 n=6)

                            │ SpanLoggerAwareCaller/with_go-kit's_Caller-10 │ SpanLoggerAwareCaller/with_dskit's_spanlogger.Caller-10 │
                            │                   allocs/op                   │              allocs/op                vs base           │
comparison-after-step-2.txt                                      9.000 ± 0%                             9.000 ± 0%  ~ (p=1.000 n=6) ¹
¹ all samples are equal
    @charleskorn
    charleskorn committed Oct 10, 2024
    Configuration menu
    Copy the full SHA
    9e662ca View commit details
    Browse the repository at this point in the history

  2. Reuse slice and use string equality check

    @charleskorn
    charleskorn committed Oct 10, 2024
    Configuration menu
    Copy the full SHA
    57c681b View commit details
    Browse the repository at this point in the history

  3. Address PR feedback: use const in test 

    Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
    @charleskorn @aknuds1
    charleskorn and aknuds1 authored Oct 10, 2024
    Configuration menu
    Copy the full SHA
    d2b6372 View commit details
    Browse the repository at this point in the history