[3.3.3 backport] CBG-5116: Diagnostic API Improvements by RIT3shSapata · Pull Request #7999 · couchbase/sync_gateway

RIT3shSapata · 2026-01-21T15:39:47Z

CBG-5116

Describe your PR here...

This PR contains all the commits of Diagnostic API

Pre-review checklist

Removed debug logging (fmt.Print, log.Print, ...)
Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Dependencies (if applicable)

Link upstream PRs
Update Go module dependencies when merged

Integration Tests

GSI=true,xattrs=true https://jenkins.sgwdev.com/job/SyncGatewayIntegration/0000/

…7932)

#7935)

)

…ad of query param (#7956)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

github-actions · 2026-01-21T15:39:59Z

Redocly previews

Copilot

Pull request overview

This PR backports diagnostic API improvements to version 3.3.3, changing the HTTP methods for sync function and import filter dry-run endpoints from GET to POST, and enhancing their functionality with support for custom functions, user context, metadata, and improved logging.

Changes:

Changed HTTP methods from GET to POST for _sync and _import_filter diagnostic endpoints
Enhanced dry-run endpoints to accept custom sync functions and import filters in request bodies
Added support for user context, user xattrs, and logging capture during dry-run operations

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
rest/routing.go	Updated HTTP methods from GET to POST for diagnostic endpoints
rest/handler.go	Added audit event handling for diagnostic server requests
rest/diagnostic_doc_api_test.go	Removed old test and added comprehensive new tests for sync function and import filter dry-run functionality
rest/diagnostic_doc_api.go	Refactored handlers to accept POST requests with enhanced payload structures and logging support
rest/audit_test.go	Added test case for diagnostic request auditing
docs/api/paths/diagnostic/keyspace-sync.yaml	Updated API documentation to reflect POST method and new request body structure
docs/api/paths/diagnostic/keyspace-import_filter.yaml	Updated API documentation to reflect POST method and new request body structure
docs/api/diagnostic.yaml	Fixed incorrect endpoint paths in API documentation
docs/api/components/schemas.yaml	Added new schema definitions for diagnostic function exception and logging
db/revision_test.go	Updated benchmark to use renamed exported function
db/revision.go	Exported `StripInternalProperties` function (renamed from `stripInternalProperties`)
db/import_test.go	Updated test calls to match refactored `EvaluateFunction` signature
db/import_listener.go	Updated calls to use renamed `UserXattrKey` method
db/import.go	Refactored import filter evaluation to support custom functions and logging
db/document.go	Updated call to use renamed `UserXattrKey` method
db/database_collection.go	Exported `UserXattrKey` method (renamed from `userXattrKey`)
db/database.go	Updated calls to use renamed methods
db/crud.go	Refactored sync function dry-run to accept custom functions and support logging
db/change_cache.go	Updated calls to use renamed `UserXattrKey` method
channels/sync_runner.go	Added `NewSyncRunnerWithLogging` to support custom logging functions
base/error.go	Added custom error types for sync function and import filter dry-run errors
base/audit_events.go	Added new audit event ID for diagnostic HTTP API requests

Copilot · 2026-01-21T15:41:08Z

rest/diagnostic_doc_api.go

-	ShouldImport bool   `json:"shouldImport"`
-	Error        string `json:"error"`
+	ShouldImport bool          `json:"shouldImport"`
+	Exception    string        `json:"exception"`


The Exception field in ImportFilterDryRun should have the omitempty JSON tag for consistency with the SyncFnDryRun struct where Exception is marked as omitempty (line 33). This ensures the field is omitted from JSON responses when empty.

Suggested change

Exception string `json:"exception"`

Exception string `json:"exception,omitempty"`

Copilot · 2026-01-21T15:41:08Z

docs/api/paths/diagnostic/keyspace-sync.yaml

+                  description: |-
+                    Optional XATTR values to include during evaluation.
+
+                    To provide the user XATTR, include a property whose name exactly matches the configured `user_xattr_key`.


The description spans multiple lines inconsistently. The continuation should be part of a single description field or properly formatted with line continuation indicators in YAML.

Suggested change

description: |-

Optional XATTR values to include during evaluation.

To provide the user XATTR, include a property whose name exactly matches the configured `user_xattr_key`.

description: Optional XATTR values to include during evaluation. To provide the user XATTR, include a property whose name exactly matches the configured `user_xattr_key`.

Copilot · 2026-01-21T15:41:09Z

rest/diagnostic_doc_api.go

+	var docid string
+
+	id, _ := syncDryRunPayload.Doc[db.BodyId].(string)
+	docid = cmp.Or(bucketDocID, id, SYNC_FN_DIAGNOSTIC_DOCID)


The use of cmp.Or to select the first non-empty string from multiple sources works correctly, but the logic would be clearer with explicit conditional checks given the significance of docid selection in this context. Consider using an explicit if-else chain to make the precedence order more apparent.

bbrks

Looks fine but I have a few questions (that may also affect main/4.0.3)

bbrks · 2026-01-22T15:20:09Z

db/import.go

 	result, err := i.Call(ctx, doc)
 	if err != nil {
-		if !dryRun {
-			base.WarnfCtx(ctx, "Unexpected error invoking import filter for document %s - processing aborted, document will not be imported.  Error: %v", base.UD(doc), err)


I've just noticed we're removing this warning logging and relying on the caller which only logs at info. Was this intentional? It seems like customers may miss this message at info level.

We do return the error back to the handler, which will send then return this error as part of exception in the response body

As discussed - there is no request here to get back to a user. An import is sourced from a doc coming over the DCP feed to Sync Gateway. Warnings are the only way of communicating this error back.

bbrks · 2026-01-22T15:21:22Z

db/import.go

 		return boolResult, nil
 	default:
-		if !dryRun {
-			base.WarnfCtx(ctx, "Import filter function returned non-boolean result %v Type: %T", result, result)


Likewise, this warning going means that customers won't see this since the caller logs errors at info level.

Similarly, the error will be sent back to the user as a exception in the response

bbrks · 2026-01-22T15:22:02Z

rest/audit_test.go

+				}
+				bodyBytes, err := json.Marshal(request)
+				require.NoError(t, err)
+				RequireStatus(t, rt.SendDiagnosticRequest(http.MethodPost, "/{{.keyspace}}/_sync", string(bodyBytes)), http.StatusOK)


Just needs the extra auth test commit pulled into this PR branch from #8006

bbrks · 2026-01-23T13:59:27Z

See #8017 for restoration of warning logs - we can cherry-pick that into this

Edit: I'll follow up with this cherry-pick so you can review independently.

RIT3shSapata and others added 13 commits January 21, 2026 18:53

CBG-4411: Improvements to Diagnostic Sync Function dry run endpoint (#…

d40928d

…7932)

CBG-4945: Fix default sync function dry run (#7937)

59bce98

CBG-4413: Add logging capture to Sync Function diagnostic API (#7938)

6b328de

CBG-4412: Add ability to pass custom import filter into diagnostic API (

d69d1ea

#7935)

Add dry-run logging output to OpenAPI specs (#7942)

63729f2

Diagnostic API: Update OpenAPI Specs (#7955)

a82255e

Diagnostic API: Tweak OpenAPI spec wording for user impersonation (#7958

95ffc89

)

CBG-5089: Add support for user defined metadata in diagnostic api (#7954

b8856db

)

CBG-5103: DocID for Diagnostic API to be passed in request body inste…

2f054b4

…ad of query param (#7956)

CBG-5101: Add support for User context in Diagnostic API (#7974)

46ca466

Correct diagnostic API routes leading underscore (#7988)

9771af1

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

CBG-5099: Fix missing doc id in doc body (#7987)

dad757b

CBG-5113: Add diagnostic server audit event (#7996)

b4b80d7

RIT3shSapata requested a review from bbrks January 21, 2026 15:39

RIT3shSapata assigned bbrks Jan 21, 2026

Copilot AI review requested due to automatic review settings January 21, 2026 15:39

lint fix

3c738d2

Copilot AI reviewed Jan 21, 2026

View reviewed changes

bbrks requested changes Jan 22, 2026

View reviewed changes

CBG-5122: Fix diagnostic audit event test (#8006)

da9d985

RIT3shSapata requested a review from bbrks January 22, 2026 16:29

bbrks assigned RIT3shSapata and unassigned bbrks Jan 23, 2026

bbrks enabled auto-merge (squash) January 26, 2026 12:12

bbrks approved these changes Jan 26, 2026

View reviewed changes

bbrks merged commit d8f9e35 into release/3.3.3 Jan 26, 2026
47 checks passed

bbrks deleted the CBG-5116 branch January 26, 2026 12:12

	Exception string `json:"exception"`
	Exception string `json:"exception,omitempty"`

Conversation

RIT3shSapata commented Jan 21, 2026

Pre-review checklist

Dependencies (if applicable)

Integration Tests

Uh oh!

github-actions bot commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Redocly previews

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

bbrks left a comment

Choose a reason for hiding this comment

Uh oh!

bbrks Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

RIT3shSapata Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

bbrks Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

bbrks Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

RIT3shSapata Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

bbrks Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

bbrks commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Jan 21, 2026 •

edited

Loading

bbrks commented Jan 23, 2026 •

edited

Loading