Include service URL and request method in r11s request errors #22851
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kian-thompson
requested review from
manishgargd,
arikt-ms,
a team,
pragya91,
markfields,
jatgarg,
tyler-cai-microsoft,
rajatch-ff and
MarioJGMsoft
and removed request for
a team
github-actions
bot
added
area: driver
msfluid-bot
reviewed
Oct 19, 2024
There was a problem hiding this comment.
Code Coverage Summary
↓ packages.drivers.driver-base.src:
Line Coverage Change: -0.37% Branch Coverage Change: 0.11%
| Metric Name | Baseline coverage | PR coverage | Coverage Diff |
|---|---|---|---|
| Branch Coverage | 85.95% | 86.06% | ↑ 0.11% |
| Line Coverage | 81.79% | 81.42% | ↓ -0.37% |
↑ packages.drivers.routerlicious-driver.src:
Line Coverage Change: 0.22% Branch Coverage Change: 2.13%
| Metric Name | Baseline coverage | PR coverage | Coverage Diff |
|---|---|---|---|
| Branch Coverage | 74.66% | 76.79% | ↑ 2.13% |
| Line Coverage | 54.33% | 54.55% | ↑ 0.22% |
Baseline commit: 199b9d0
Baseline build: 302875
Happy Coding!!
Code coverage comparison check passed!!
⯅ @fluid-example/bundle-size-tests: +910 Bytes
Baseline commit: 199b9d0 |
arikt-ms
reviewed
Oct 21, 2024
| if (path !== undefined) { | ||
| // Extract the first portion of the path and explicitly match it to known path names | ||
| const pathMatch = path.match(/^\/?([^/]+)/); | ||
| if (pathMatch && [socketIoPath, "repos", "deltas", "documents"].includes(pathMatch[1])) { |
There was a problem hiding this comment.
because there are multiple urls that start with the same path segment (i.e. "documents", "repos") and have the same HTTP method we might need some additional information logged to be able to differentiate between those (i.e. total segment length, etc.). @manishgargd to comment if that is really necessary for this task.
arikt-ms
reviewed
Oct 21, 2024
|
I assume the response status code and method are already being logged, correct ? |
kian-thompson
added a commit
that referenced
this pull request
Oct 31, 2024
We can add more portions of the URL path on r11s request errors. To protect against logging IDs in the URL, any path portion that does not explicitly match the list of known paths will be replaced with `REDACTED`. Continuation of #22851 Fixes [AB#22156](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/22156)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Troubleshooting live-site issues and customer reported issues will become easier. Ex: If Container creation failures have service URL and response code, it'll be easier to identify point of failure and come-up with a fix to mitigate/fix the incident.
This gives AFR a good idea about the APIs which are failing, which cluster it was going to, and which cluster group it was going to. It helps AFR correlate the errors on the server better.
Examples
Here are some examples of request calls. Only the bolded parts can be logged, and the first line of each entry below is just the type of request being made (no need to log it):
CreateDocument (or CreateContainer)
POST https://us.fluidrelay.azure.com/documents/someId
Connect
GET https://alfred.southcentralus-1.fluidrelay.azure.com/socket.io/?documentId=someId&tenantId=someId&EIO=4&transport=websocket
GetDelta
GET https://alfred.southcentralus-1.fluidrelay.azure.com/deltas/someId/someId?from=0&to=2001
GetSummary
GET https://historian.southcentralus-1.fluidrelay.azure.com/repos/someId/git/summaries/latest?token=someToken&disableCache=true
PostSummary
POST https://historian.southcentralus-1.fluidrelay.azure.com/repos/someId/git/summaries?token=someToken
FindSession – First Request
POST https://alfred.southcentralus-1.fluidrelay.azure.com/documents/someId/session/someId
Fixes AB#18452