-
Notifications
You must be signed in to change notification settings - Fork 139
CBG-4497 centralize blip correlation ids #7498
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has subtly changed the format of BLIP context IDs which is probably undesirable.
I think it's just a case of passing the context ID through FormatBlipContextID()
?
Otherwise looks OK to me
Compare before:
2025-04-25T13:04:59.891+01:00 [INF] HTTP: c:#005 db:db1 GET /db1/_blipsync (as <ud>demo</ud>)
2025-04-25T13:04:59.891+01:00 [INF] HTTP+: c:[1e02c999] db:db1 #005: --> 101 [1e02c999] Upgraded to WebSocket protocol BLIP_3+CBMobile_2 (as <ud>demo</ud>) (0.0 ms)
2025-04-25T13:04:59.891+01:00 [INF] WS: c:#005 db:db1 Start BLIP/Websocket handler
2025-04-25T13:04:59.892+01:00 [INF] SyncMsg: c:[1e02c999] db:db1 col:_default #1: Type:changes #Changes:200
2025-04-25T13:04:59.950+01:00 [DBG] SyncMsg+: c:[1e02c999] db:db1 #1: Type:changes --> OK Time:58.634333ms
After:
2025-04-25T13:08:14.918+01:00 [INF] HTTP: c:#007 db:db1 GET /db1/_blipsync (as <ud>demo</ud>)
2025-04-25T13:08:14.918+01:00 [INF] HTTP+: c:[47f59398] db:db1 #007: --> 101 [47f59398] Upgraded to WebSocket protocol BLIP_3+CBMobile_2 (as <ud>demo</ud>) (0.0 ms)
2025-04-25T13:08:14.918+01:00 [INF] WS: c:47f59398 db:db1 Start BLIP/Websocket handler
2025-04-25T13:08:14.918+01:00 [INF] SyncMsg: c:47f59398 db:db1 col:_default #1: Type:changes #Changes:200
2025-04-25T13:08:14.976+01:00 [DBG] SyncMsg+: c:47f59398 db:db1 #1: Type:changes --> OK Time:57.73725ms
Note: /ks/_blipsync endpoint and active replicator have different formats
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good sans honk
Co-authored-by: Ben Brooks <[email protected]>
db/blip.go
Outdated
@@ -12,6 +12,7 @@ package db | |||
|
|||
import ( | |||
"context" | |||
"fmt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"fmt" |
CBG-4497 centralize blip correlation ids
Force assignment of blip correlation IDs to match the blip context for _blipsync and ISGR. The important change is in
NewSGBlipContextWithProtocols
to havebc.Logger = defaultBlipLogger(ctx)
use the right correlation id.ISGR and _blipsync did reset the context on
BlipSyncContext
after its construction, but this seems confusing and error prone, so I just forced setting the correlation id inNewBlipSyncContext
constructor. I could leave the this part of the code alone and just do the change inNewSGBlipContextWithProtocols
Pre-review checklist
fmt.Print
,log.Print
, ...)base.UD(docID)
,base.MD(dbName)
)docs/api
Integration Tests
GSI=true,xattrs=true
https://jenkins.sgwdev.com/job/SyncGateway-Integration/3063/