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 missing logs for HTTP requests #1153

Merged

Conversation

josecelano
Copy link
Member

@josecelano josecelano commented Dec 23, 2024

Relates to:

Some log lines (tracing spans) were missing. In addition to that, we were using new spans to log requests and responses instead of using the span provided by the TraceLayer. That enables you to join all the log events related to the same request.

This fix is needed to continue with this issue: #1150

Because it allows the use of a "request-id" header to identify logs.

We can write log assertions by matching lines with the request-id used in the request.

For example, if you make this request:

curl -H "x-request-id: YOUR_REQUEST_ID" http://0.0.0.0:1212/api/v1/stats?token=InvalidToken

This is the new output (which was missing before this change):

2024-12-23T17:53:06.530704Z  INFO request{method=GET uri=/api/v1/stats?token=InvalidToken version=HTTP/1.1}: API: request method=GET uri=/api/v1/stats?token=InvalidToken request_id=YOUR_REQUEST_ID
2024-12-23T17:53:06.530777Z ERROR request{method=GET uri=/api/v1/stats?token=InvalidToken version=HTTP/1.1}: API: response latency_ms=0 status_code=500 Internal Server Error request_id=YOUR_REQUEST_ID
2024-12-23T17:53:06.530785Z ERROR request{method=GET uri=/api/v1/stats?token=InvalidToken version=HTTP/1.1}: API: response failed failure_classification=Status code: 500 Internal Server Error latency=0 ms

As you can see, now we have the "request_id=YOUR_REQUEST_ID" field, which can be used to identify the test that made the request and write a log assertion for that concrete expected line.

Some tracing log spans were missing. In adition to that we were using
new spans to log requests and responses instead of using the span
provided by the TraceLayer. That enables to join all the log events
related tho the same requests.

This fix is needed to contunie with this issue:

torrust#1150

Becuase it allos to use a "request-id" header to identify logs.

We can write log assertions by mathich lines with the request-id used in
the request.

For example, it yo make this request:

```console
curl -H "x-request-id: YOUR_REQUEST_ID" http://0.0.0.0:1212/api/v1/stats?token=InvalidToken
```

This is the new output (which was missing before this change):

```output
2024-12-23T17:53:06.530704Z  INFO request{method=GET uri=/api/v1/stats?token=InvalidToken version=HTTP/1.1}: API: request method=GET uri=/api/v1/stats?token=InvalidToken request_id=YOUR_REQUEST_ID
2024-12-23T17:53:06.530777Z ERROR request{method=GET uri=/api/v1/stats?token=InvalidToken version=HTTP/1.1}: API: response latency_ms=0 status_code=500 Internal Server Error request_id=YOUR_REQUEST_ID
2024-12-23T17:53:06.530785Z ERROR request{method=GET uri=/api/v1/stats?token=InvalidToken version=HTTP/1.1}: API: response failed failure_classification=Status code: 500 Internal Server Error latency=0 ms
```

As you can see. now we have the "request_id=YOUR_REQUEST_ID" field which
can be used to identify the test that made the request.
@josecelano
Copy link
Member Author

ACK 97233f5

Copy link

codecov bot commented Dec 23, 2024

Codecov Report

Attention: Patch coverage is 60.00000% with 24 lines in your changes missing coverage. Please review.

Project coverage is 75.98%. Comparing base (0f40030) to head (97233f5).
Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
src/servers/health_check_api/server.rs 50.00% 10 Missing ⚠️
src/servers/http/v1/routes.rs 50.00% 10 Missing ⚠️
src/servers/apis/routes.rs 80.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1153      +/-   ##
===========================================
- Coverage    76.02%   75.98%   -0.05%     
===========================================
  Files          174      174              
  Lines        11583    11601      +18     
  Branches     11583    11601      +18     
===========================================
+ Hits          8806     8815       +9     
- Misses        2608     2618      +10     
+ Partials       169      168       -1     

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

@josecelano josecelano merged commit 64039e5 into torrust:develop Dec 23, 2024
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant