[ms-bing-ads-audiences] Add temporary debug logging of API responses#3877
Open
harsh-joshi99 wants to merge 4 commits into
Open
[ms-bing-ads-audiences] Add temporary debug logging of API responses#3877harsh-joshi99 wants to merge 4 commits into
harsh-joshi99 wants to merge 4 commits into
Conversation
Non-bulk destinations don't persist responses from the destination, which makes debugging the MS Bing Ads Audiences sync difficult — in particular getting Microsoft's tracking id needed to raise Bing Ads support tickets. Adds temporary debug logging gated behind the 'actions-ms-bing-ads-audiences-debug-logging' feature flag (off by default). Logs only successful responses: Microsoft's tracking id (from response header or body), non-sensitive request metadata (action, audience id, identifier type, item count) and a redacted PartialErrors summary (codes/index only). No PII: CustomerListItems (hashed emails / unhashed CRM ids) and the PartialError free-text fields (Message/Details/FieldPath) are never logged. No behavior change: logging is a no-op unless the flag is on, and the error path flows through handleHttpError unchanged. Marked TEMPORARY for removal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds feature-flagged, temporary debug logging for MS Bing Ads Audiences CustomerListUserData/Apply responses to help capture Microsoft tracking/request IDs for support/debugging without logging PII.
Changes:
- Gate debug logging behind
actions-ms-bing-ads-audiences-debug-loggingand include trackingId, request metadata, and redacted PartialErrors summary. - Thread
features+loggerthroughperform/performBatchintosyncUserto enable conditional logging. - Add unit tests covering flag on/off, tracking id extraction (header/body), redaction, and no logging on error path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts | Adds gated debug logging utilities and emits redacted logs on successful Apply calls. |
| packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/tests/index.test.ts | Adds test coverage for the new debug logging behavior and safety constraints. |
logBingAdsResponse runs inside syncUser's try/catch after Bing has already accepted the records. A throwing or partial logger would be caught and rethrown — failing an otherwise-successful (batch) delivery and triggering a duplicate re-send on retry. Wrap the log body in its own try/catch so best-effort debug logging can never alter delivery semantics. Adds a test asserting a throwing logger.info still returns 200. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines
+73
to
+80
| const logBingAdsResponse = ( | ||
| logger: Logger | undefined, | ||
| debugLogging: boolean, | ||
| action: string, | ||
| audienceId: string, | ||
| sentPayload: SyncAudiencePayload, | ||
| response: ModifiedResponse | ||
| ): void => { |
Comment on lines
+86
to
+91
| const { CustomerListItemSubType, CustomerListItems } = sentPayload.CustomerListUserData | ||
| const partialErrors = (response.data as { PartialErrors?: PartialError[] } | undefined)?.PartialErrors | ||
| const line = | ||
| `[ms-bing-ads-audiences][DEBUG] ${action} audienceId=${audienceId} status=${response.status} ` + | ||
| `trackingId=${extractTrackingId(response)} identifierType=${CustomerListItemSubType} ` + | ||
| `itemCount=${CustomerListItems.length} partialErrors=${summarizeErrors(partialErrors)}` |
…Grafana The delivery runtime's logger filters out info-level lines, so logger.info was accepted but never shipped to the log pipeline — the debug output never appeared in Grafana despite the flag being on and performBatch running. Switch to logger?.warn?.() (warn is the lowest level that reliably ships, matching hubspot/aws-kinesis) and use the optional-call idiom so a partial logger no-ops rather than throwing. Tests updated to assert on warn. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment on lines
+60
to
+65
| const extractTrackingId = (response: ModifiedResponse): string => { | ||
| const header = response.headers?.get('trackingid') || response.headers?.get('x-ms-trackingid') | ||
| if (header) return header | ||
| const body = response.data as { TrackingId?: string } | undefined | ||
| return body?.TrackingId || 'none' | ||
| } |
Comment on lines
+73
to
+80
| const logBingAdsResponse = ( | ||
| logger: Logger | undefined, | ||
| debugLogging: boolean, | ||
| action: string, | ||
| audienceId: string, | ||
| sentPayload: SyncAudiencePayload, | ||
| response: ModifiedResponse | ||
| ): void => { |
This file contains hidden or 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
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.
Summary
Non-bulk destinations don't persist the responses received from the destination, which makes debugging the MS Bing Ads Audiences sync difficult — in particular, getting Microsoft's tracking/request id needed to raise Bing Ads support tickets.
This PR adds temporary debug logging, gated behind the
actions-ms-bing-ads-audiences-debug-loggingfeature flag (off by default). It logs only successful responses from eachCustomerListUserData/Applycall:TrackingId/x-ms-trackingidresponse header, or a top-level bodyTrackingId) — for quoting to Bing Ads supportCustomerListItemsPartialErrorssummary (ErrorCode/Code/Index/Typeonly)This is a deliberately lean alternative to #3836.
Safety
CustomerListItems(hashed emails / unhashed CRM ids) are never logged, and thePartialErrorfree-text fields (Message/Details/FieldPath) — which can echo back an identifier — are dropped.handleHttpErrorunchanged. The whole log line is capped at 4096 chars.Intended to be removed once debugging is complete — every added block is marked
// TEMPORARY.Flagon: https://flagon.segment.com/families/centrifuge-destinations/gates/actions-ms-bing-ads-audiences-debug-logging
Testing
npx jest src/destinations/ms-bing-ads-audiences)Security Review
🤖 Generated with Claude Code