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

feat: introducing chi middleware for warehouse #4010

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

achettyiitr
Copy link
Member

@achettyiitr achettyiitr commented Oct 25, 2023

Description

  • Using chi.middleWare to introduce HTTP API response times for the warehouse. This will help us provide better observability around the HTTP requests.

Linear Ticket

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

warehouse/api/http.go Outdated Show resolved Hide resolved
@@ -127,6 +129,9 @@ func NewApi(

func (a *Api) Start(ctx context.Context) error {
srvMux := chi.NewRouter()
srvMux.Use(
chiware.StatMiddleware(ctx, srvMux, a.statsFactory, "warehouse"),
Copy link
Collaborator

@fracasula fracasula Oct 25, 2023

Choose a reason for hiding this comment

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

The 2nd argument doesn't seem to be used internally, @atzoum can we remove it from the kit?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch @fracasula . Middleware doesn't need to have access to the router.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@codecov
Copy link

codecov bot commented Oct 25, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (28497cf) 71.39% compared to head (4bde439) 71.39%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4010   +/-   ##
=======================================
  Coverage   71.39%   71.39%           
=======================================
  Files         371      371           
  Lines       54472    54475    +3     
=======================================
+ Hits        38889    38895    +6     
+ Misses      13270    13267    -3     
  Partials     2313     2313           
Files Coverage Δ
warehouse/api/http.go 78.94% <100.00%> (+0.22%) ⬆️

... and 11 files with indirect coverage changes

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

@achettyiitr achettyiitr merged commit 4d9f9b3 into master Oct 26, 2023
35 checks passed
@achettyiitr achettyiitr deleted the feat.http-middleware branch October 26, 2023 06:14
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.

3 participants