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.

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

🐛 fix(middleware/logger): default latency output format #2580

Merged
merged 4 commits into from
Aug 17, 2023

Conversation

sixcolors
Copy link
Member

@sixcolors sixcolors commented Aug 16, 2023

Description

fixes middleware/logger default latency output format to match TagLatency

Fixes #2561

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I tried to make my code as fast as possible with as few allocations as possible

Commit formatting:

Use emojis on commit messages so it provides an easy way of identifying the purpose or intention of a commit. Check out the emoji cheatsheet here: https://gitmoji.carloscuesta.me/

@gaby
Copy link
Member

gaby commented Aug 16, 2023

Any ideas why the CI is failing?

@sixcolors
Copy link
Member Author

Any ideas why the CI is failing?

@gaby windows VM is slow.

=== CONT  Test_Logger_WithLatency_DefaultFormat
    logger_test.go:303: 
        Test:            Test_Logger_WithLatency_DefaultFormat
        Trace:           logger_test.go:303
        Description:     Expected latency to be in µs, got      1.5727ms
        Expect:          false     (bool)
        Result:          true      (bool)
=== CONT  Test_Logger_WithLatency
    logger_test.go:251: 
        Test:            Test_Logger_WithLatency
        Trace:           logger_test.go:251
        Description:     Expected latency to be in µs, got      1.5727ms
        Expect:          false     (bool)
        Result:          true      (bool)
--- FAIL: Test_Logger_WithLatency_DefaultFormat (0.00s)

@gaby
Copy link
Member

gaby commented Aug 17, 2023

Any ideas why the CI is failing?

@gaby windows VM is slow.

=== CONT  Test_Logger_WithLatency_DefaultFormat
    logger_test.go:303: 
        Test:            Test_Logger_WithLatency_DefaultFormat
        Trace:           logger_test.go:303
        Description:     Expected latency to be in µs, got      1.5727ms
        Expect:          false     (bool)
        Result:          true      (bool)
=== CONT  Test_Logger_WithLatency
    logger_test.go:251: 
        Test:            Test_Logger_WithLatency
        Trace:           logger_test.go:251
        Description:     Expected latency to be in µs, got      1.5727ms
        Expect:          false     (bool)
        Result:          true      (bool)
--- FAIL: Test_Logger_WithLatency_DefaultFormat (0.00s)

@sixcolors I wonder if removing t.Parallel() for those helps?

Trying to make windows CI pass....
@sixcolors
Copy link
Member Author

@gaby could be related to golang/go#29485

@gaby
Copy link
Member

gaby commented Aug 17, 2023

@gaby could be related to golang/go#29485

Yeah, may need to update the tests then

@gaby
Copy link
Member

gaby commented Aug 17, 2023

Unrelated MacOS failure this time

Copy link
Member

@gaby gaby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@ReneWerner87 ReneWerner87 merged commit c3ae066 into gofiber:master Aug 17, 2023
17 checks passed
@sixcolors sixcolors deleted the 2561-logger-default-latency branch March 27, 2024 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🚀 [Feature]: Log show lowerst time-unit that makes sense [#2258]
3 participants