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

Small log additions and moving metrics from Seconds to Milliseconds #80

Merged
merged 4 commits into from
Nov 23, 2023

Conversation

madflojo
Copy link
Member

@madflojo madflojo commented Nov 23, 2023

This removes headers from logs, as sometimes headers might hold secrets that shouldn't be logged. Also adds time to logs.

Summary by CodeRabbit

  • Refactor

    • Improved precision in logging by displaying durations in milliseconds for both HTTP requests and WASM module execution.
  • Bug Fixes

    • Enhanced logging for PProf requests to include duration and status messages when PProf is disabled.

Copy link
Contributor

coderabbitai bot commented Nov 23, 2023

Warning

Rate Limit Exceeded

@madflojo has exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 10 minutes and 24 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
Please see our FAQ for further information.

Commits Files that changed from the base of the PR and between 02a1ab2 and 8312882.

Your existing content is already well-structured and captures the required information. I'll repeat it verbatim without additional text:

Walkthrough

The recent updates to the server application focus on enhancing the precision of logging and observation metrics by shifting from seconds to milliseconds. This change affects how the application logs HTTP requests, PProf handling, and the execution of WebAssembly (WASM) modules, providing more granular insight into the performance and duration of these operations.

Changes

File Path Change Summary
.../server.go Updated HTTP request and PProf logging to include request duration in milliseconds; added detailed logging for WASM execution time in milliseconds.
.../app.go Modified callback execution time logging and task observation to use milliseconds for better precision.

Poem

As the leaves fall in a dance, 🍂
Our code now measures time with a glance. ⏱️
Milliseconds in logs, a precision advance,
A rabbit's touch in the autumn expanse. 🐇🌟


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 9a525ae and fe778de.
Files selected for processing (1)
  • pkg/app/server.go (3 hunks)
Additional comments: 1
pkg/app/server.go (1)
  • 55-64: The changes correctly log an attempt to access PProf when it is disabled and respond with a StatusForbidden. This is a good security practice as it logs unauthorized access attempts. Ensure that the enable_pprof configuration is properly documented and that the system administrators are aware of this feature.

pkg/app/server.go Outdated Show resolved Hide resolved
pkg/app/server.go Outdated Show resolved Hide resolved
Copy link

codecov bot commented Nov 23, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (9a525ae) 83.03% compared to head (8312882) 83.14%.

Files Patch % Lines
pkg/app/app.go 55.55% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #80      +/-   ##
==========================================
+ Coverage   83.03%   83.14%   +0.11%     
==========================================
  Files          12       12              
  Lines        1167     1181      +14     
==========================================
+ Hits          969      982      +13     
- Misses        152      153       +1     
  Partials       46       46              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@madflojo madflojo changed the title Adding some small log additions Small log additions and moving metrics from Seconds to Milliseconds Nov 23, 2023
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between fe778de and a4a720a.
Files selected for processing (2)
  • pkg/app/app.go (2 hunks)
  • pkg/app/server.go (2 hunks)
Additional comments: 3
pkg/app/server.go (3)
  • 48-52:
    The change to include "received" in the log message is implemented correctly. However, ensure that the removal of headers from the logs is consistent with the pull request's goal of not logging sensitive information.

  • 55-67:
    The change to log the duration in milliseconds when PProf is disabled is implemented correctly. However, ensure that the Observe method is correctly handling milliseconds since this is a change from the previous behavior.

  • 138-159:
    The changes to log the execution duration of WASM modules in milliseconds and to include additional details such as the "module" and "handler" are implemented correctly. Ensure that the Observe method is correctly handling milliseconds and that the logging changes do not introduce any performance issues.

pkg/app/app.go Outdated Show resolved Hide resolved
pkg/app/app.go Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between a4a720a and 02a1ab2.
Files selected for processing (1)
  • pkg/app/app.go (2 hunks)
Additional comments: 2
pkg/app/app.go (2)
  • 328-357:
    The changes to log and observe callback execution times in milliseconds are correctly implemented and consistent with the pull request's goals.
    [APROVED]

  • 511-517:
    The implementation of task execution time observation in milliseconds after error checking is correct and does not result in double counting.

@madflojo madflojo merged commit e9b2592 into tarmac-project:main Nov 23, 2023
11 checks passed
@madflojo madflojo deleted the logs branch November 23, 2023 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant