From c32323909297aa9a12af5843476eabfaf75f4414 Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Tue, 14 Jul 2026 13:26:34 +0530 Subject: [PATCH 1/3] [ms-bing-ads-audiences] Add temporary debug logging of API responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../syncAudiences/__tests__/index.test.ts | 124 ++++++++++++++++++ .../syncAudiences/index.ts | 65 ++++++++- 2 files changed, 182 insertions(+), 7 deletions(-) diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts index 596193a2677..ceea2574272 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts @@ -264,6 +264,130 @@ describe('MS Bing Ads Audiences syncAudiences', () => { expect(payloadArg[0].email).toBe('add1@segment.com') }) + describe('debug logging (actions-ms-bing-ads-audiences-debug-logging flag)', () => { + const DEBUG_FLAG = 'actions-ms-bing-ads-audiences-debug-logging' + + const makeLogger = () => ({ info: jest.fn(), error: jest.fn() } as any) + + const addEvent = () => + createTestEvent({ + type: 'identify', + properties: { aud_key: true }, + context: { traits: { email: 'demo@segment.com' } } + }) + + it('does not log when the flag is off', async () => { + nock(BASE_URL).post('/CustomerListUserData/Apply').reply(200, {}) + const logger = makeLogger() + + await testDestination.testAction('syncAudiences', { + event: addEvent(), + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: false } + }) + + expect(logger.info).not.toHaveBeenCalled() + }) + + it('logs the tracking id and metadata when on, without leaking hashed identifiers', async () => { + nock(BASE_URL) + .post('/CustomerListUserData/Apply') + .reply(200, { PartialErrors: [] }, { TrackingId: 'abc-123-track' }) + const logger = makeLogger() + + await testDestination.testAction('syncAudiences', { + event: addEvent(), + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + expect(logger.info).toHaveBeenCalledTimes(1) + const logged = (logger.info as jest.Mock).mock.calls[0][0] as string + expect(logged).toContain('[ms-bing-ads-audiences][DEBUG]') + expect(logged).toContain('trackingId=abc-123-track') + expect(logged).toContain('identifierType=Email') + expect(logged).toContain('itemCount=1') + // The hashed identifier must never be logged. + expect(logged).not.toContain('5a95f052958dac8ed1d66d74eb481b3ccdbbc953b583c5ff0325be6b091d6281') + }) + + it('falls back to a body-level tracking id when no header is present', async () => { + nock(BASE_URL).post('/CustomerListUserData/Apply').reply(200, { TrackingId: 'body-track-789', PartialErrors: [] }) + const logger = makeLogger() + + await testDestination.testAction('syncAudiences', { + event: addEvent(), + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + const logged = (logger.info as jest.Mock).mock.calls[0][0] as string + expect(logged).toContain('trackingId=body-track-789') + }) + + it('redacts PartialError free-text fields that can echo identifiers', async () => { + nock(BASE_URL) + .post('/CustomerListUserData/Apply') + .reply(200, { + PartialErrors: [ + { + ErrorCode: 'InvalidCustomerListItem', + Code: 4001, + Index: 0, + Type: 'BatchError', + Message: 'Invalid value crm_secret_12345', + Details: 'crm_secret_12345', + FieldPath: 'CustomerListItems[0]=crm_secret_12345' + } + ] + }) + const logger = makeLogger() + + await testDestination.testBatchAction('syncAudiences', { + events: [addEvent()], + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + const logged = (logger.info as jest.Mock).mock.calls[0][0] as string + expect(logged).toContain('InvalidCustomerListItem') + // The free-text fields (and any identifier they echo) must not be logged. + expect(logged).not.toContain('crm_secret_12345') + expect(logged).not.toContain('Message') + expect(logged).not.toContain('FieldPath') + }) + + it('does not log on the error path (only success responses are logged)', async () => { + nock(BASE_URL).post('/CustomerListUserData/Apply').reply(500, { message: 'boom' }) + const logger = makeLogger() + + const response = await testDestination.testBatchAction('syncAudiences', { + events: [addEvent()], + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + expect(logger.info).not.toHaveBeenCalled() + expect(utils.handleHttpError).toHaveBeenCalled() + expect(response[0].status).toBe(500) + }) + }) + it('should throw non-HTTP errors in batch mode', async () => { // Create a custom error that is NOT an HTTPError const customError = new Error('Custom non-HTTP error') diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts index e93d5fe65d8..2ef4e02579f 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts @@ -1,4 +1,4 @@ -import { ActionDefinition, RequestClient, HTTPError } from '@segment/actions-core' +import { ActionDefinition, RequestClient, HTTPError, Logger, ModifiedResponse } from '@segment/actions-core' import { MultiStatusResponse } from '@segment/actions-core' import type { Settings } from '../generated-types' import type { Payload } from './generated-types' @@ -20,7 +20,7 @@ import { handleHttpError, categorizePayloadByAction } from '../utils' -import { Identifier, SyncAudiencePayload } from '../types' +import { Identifier, SyncAudiencePayload, PartialError } from '../types' const action: ActionDefinition = { title: 'Sync Audiences', @@ -36,15 +36,58 @@ const action: ActionDefinition = { batch_size, computation_class }, - perform: async (request, { payload }) => { - return await syncUser(request, [payload], false) + perform: async (request, { payload, features, logger }) => { + return await syncUser(request, [payload], false, isDebugLoggingEnabled(features), logger) }, - performBatch: async (request, { payload }) => { - return await syncUser(request, payload, true) + performBatch: async (request, { payload, features, logger }) => { + return await syncUser(request, payload, true, isDebugLoggingEnabled(features), logger) } } +// TEMPORARY DEBUG LOGGING — gated behind the 'actions-ms-bing-ads-audiences-debug-logging' feature +// flag (off by default). Logs Microsoft's tracking id (needed to file Bing Ads support tickets) +// plus non-sensitive request metadata and a redacted error summary. It intentionally does NOT log +// CustomerListItems (hashed emails / unhashed CRM ids) or the PartialError free-text fields +// (Message/Details/FieldPath), which can echo back an identifier. Remove once debugging is done. +const DEBUG_LOGGING_FLAG = 'actions-ms-bing-ads-audiences-debug-logging' + +const isDebugLoggingEnabled = (features: Record | undefined): boolean => + Boolean(features?.[DEBUG_LOGGING_FLAG]) + +// Microsoft returns its request/tracking id as a response header (and sometimes echoes it in the +// body). It's a short opaque id, not PII. +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' +} + +// Reduce PartialErrors to codes + index; the free-text fields can echo back the offending +// identifier so they are dropped. +const summarizeErrors = (errors: PartialError[] | undefined): string => + JSON.stringify((errors ?? []).map((e) => ({ ErrorCode: e.ErrorCode, Code: e.Code, Index: e.Index, Type: e.Type }))) + +// TEMPORARY: log a redacted, PII-safe summary of a successful Bing Ads response. +const logBingAdsResponse = ( + logger: Logger | undefined, + debugLogging: boolean, + action: string, + audienceId: string, + sentPayload: SyncAudiencePayload, + response: ModifiedResponse +): void => { + if (!debugLogging || !logger) return + 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)}` + logger.info(line.slice(0, 4096)) +} + /** * Synchronizes user audience data with Microsoft Bing Ads. * @@ -58,7 +101,13 @@ const action: ActionDefinition = { * @returns A promise that resolves to a `MultiStatusResponse` object summarizing the results. * @throws Will throw an error if a non-batch operation fails, or rethrows non-HTTP errors in batch mode. */ -const syncUser = async (request: RequestClient, payload: Payload[], isBatch: boolean) => { +const syncUser = async ( + request: RequestClient, + payload: Payload[], + isBatch: boolean, + debugLogging = false, + logger?: Logger +) => { const msResponse = new MultiStatusResponse() if (!Array.isArray(payload) || payload.length === 0) { @@ -87,10 +136,12 @@ const syncUser = async (request: RequestClient, payload: Payload[], isBatch: boo // Send data to Microsoft Bing Ads for both Add and Remove actions if they have entries if (addMap.size > 0) { const response = await sendDataToMicrosoftBingAds(request, addPayload) + logBingAdsResponse(logger, debugLogging, 'Add', audienceId, addPayload, response) handleMultistatusResponse(msResponse, response, addItems, addMap, payload, isBatch) } if (removeMap.size > 0) { const response = await sendDataToMicrosoftBingAds(request, removePayload) + logBingAdsResponse(logger, debugLogging, 'Remove', audienceId, removePayload, response) handleMultistatusResponse(msResponse, response, removeItems, removeMap, payload, isBatch) } } catch (error) { From dc27fa8e3b091dd9fcd0dbc76fe7939d2b1ed711 Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Tue, 14 Jul 2026 18:16:50 +0530 Subject: [PATCH 2/3] [ms-bing-ads-audiences] Isolate debug logging from delivery control flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../syncAudiences/__tests__/index.test.ts | 21 +++++++++++++++++++ .../syncAudiences/index.ts | 21 ++++++++++++------- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts index ceea2574272..7b3943bd146 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts @@ -369,6 +369,27 @@ describe('MS Bing Ads Audiences syncAudiences', () => { expect(logged).not.toContain('FieldPath') }) + it('does not let a throwing logger break the delivery', async () => { + // Debug logging runs after Bing has accepted the records; a throwing logger must not + // fail the delivery (which would trigger a duplicate re-send on retry). + nock(BASE_URL).post('/CustomerListUserData/Apply').reply(200, { PartialErrors: [] }) + const logger = makeLogger() + ;(logger.info as jest.Mock).mockImplementation(() => { + throw new Error('logger down') + }) + + const response = await testDestination.testAction('syncAudiences', { + event: addEvent(), + mapping: baseMapping, + useDefaultMappings: true, + settings, + logger, + features: { [DEBUG_FLAG]: true } + }) + + expect(response[0].status).toBe(200) + }) + it('does not log on the error path (only success responses are logged)', async () => { nock(BASE_URL).post('/CustomerListUserData/Apply').reply(500, { message: 'boom' }) const logger = makeLogger() diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts index 2ef4e02579f..d1f92e6cf9b 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts @@ -79,13 +79,20 @@ const logBingAdsResponse = ( response: ModifiedResponse ): void => { if (!debugLogging || !logger) return - 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)}` - logger.info(line.slice(0, 4096)) + // Isolate from delivery control flow: this runs inside the syncUser try/catch after Bing has + // already accepted the records, so a throwing/partial logger must never fail an otherwise + // successful (batch) delivery or trigger a duplicate re-send on retry. + try { + 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)}` + logger.info(line.slice(0, 4096)) + } catch { + // Best-effort debug logging — intentionally swallowed. + } } /** From 449d15db2f02166799f5c23344b1e6bbb4cd9de7 Mon Sep 17 00:00:00 2001 From: Harsh Joshi Date: Wed, 15 Jul 2026 23:58:51 +0530 Subject: [PATCH 3/3] [ms-bing-ads-audiences] Log debug output at warn level so it reaches Grafana MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../syncAudiences/__tests__/index.test.ts | 16 ++++++++-------- .../ms-bing-ads-audiences/syncAudiences/index.ts | 5 ++++- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts index 7b3943bd146..928ec308759 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/__tests__/index.test.ts @@ -267,7 +267,7 @@ describe('MS Bing Ads Audiences syncAudiences', () => { describe('debug logging (actions-ms-bing-ads-audiences-debug-logging flag)', () => { const DEBUG_FLAG = 'actions-ms-bing-ads-audiences-debug-logging' - const makeLogger = () => ({ info: jest.fn(), error: jest.fn() } as any) + const makeLogger = () => ({ info: jest.fn(), warn: jest.fn(), error: jest.fn() } as any) const addEvent = () => createTestEvent({ @@ -289,7 +289,7 @@ describe('MS Bing Ads Audiences syncAudiences', () => { features: { [DEBUG_FLAG]: false } }) - expect(logger.info).not.toHaveBeenCalled() + expect(logger.warn).not.toHaveBeenCalled() }) it('logs the tracking id and metadata when on, without leaking hashed identifiers', async () => { @@ -307,8 +307,8 @@ describe('MS Bing Ads Audiences syncAudiences', () => { features: { [DEBUG_FLAG]: true } }) - expect(logger.info).toHaveBeenCalledTimes(1) - const logged = (logger.info as jest.Mock).mock.calls[0][0] as string + expect(logger.warn).toHaveBeenCalledTimes(1) + const logged = (logger.warn as jest.Mock).mock.calls[0][0] as string expect(logged).toContain('[ms-bing-ads-audiences][DEBUG]') expect(logged).toContain('trackingId=abc-123-track') expect(logged).toContain('identifierType=Email') @@ -330,7 +330,7 @@ describe('MS Bing Ads Audiences syncAudiences', () => { features: { [DEBUG_FLAG]: true } }) - const logged = (logger.info as jest.Mock).mock.calls[0][0] as string + const logged = (logger.warn as jest.Mock).mock.calls[0][0] as string expect(logged).toContain('trackingId=body-track-789') }) @@ -361,7 +361,7 @@ describe('MS Bing Ads Audiences syncAudiences', () => { features: { [DEBUG_FLAG]: true } }) - const logged = (logger.info as jest.Mock).mock.calls[0][0] as string + const logged = (logger.warn as jest.Mock).mock.calls[0][0] as string expect(logged).toContain('InvalidCustomerListItem') // The free-text fields (and any identifier they echo) must not be logged. expect(logged).not.toContain('crm_secret_12345') @@ -374,7 +374,7 @@ describe('MS Bing Ads Audiences syncAudiences', () => { // fail the delivery (which would trigger a duplicate re-send on retry). nock(BASE_URL).post('/CustomerListUserData/Apply').reply(200, { PartialErrors: [] }) const logger = makeLogger() - ;(logger.info as jest.Mock).mockImplementation(() => { + ;(logger.warn as jest.Mock).mockImplementation(() => { throw new Error('logger down') }) @@ -403,7 +403,7 @@ describe('MS Bing Ads Audiences syncAudiences', () => { features: { [DEBUG_FLAG]: true } }) - expect(logger.info).not.toHaveBeenCalled() + expect(logger.warn).not.toHaveBeenCalled() expect(utils.handleHttpError).toHaveBeenCalled() expect(response[0].status).toBe(500) }) diff --git a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts index d1f92e6cf9b..d6f47380a15 100644 --- a/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts +++ b/packages/destination-actions/src/destinations/ms-bing-ads-audiences/syncAudiences/index.ts @@ -89,7 +89,10 @@ const logBingAdsResponse = ( `[ms-bing-ads-audiences][DEBUG] ${action} audienceId=${audienceId} status=${response.status} ` + `trackingId=${extractTrackingId(response)} identifierType=${CustomerListItemSubType} ` + `itemCount=${CustomerListItems.length} partialErrors=${summarizeErrors(partialErrors)}` - logger.info(line.slice(0, 4096)) + // Emit at warn: the delivery runtime's logger filters out info-level lines, so info never + // reaches the log pipeline. warn is the lowest level that reliably ships. Uses the optional + // ?.warn?.() call idiom so a partial logger no-ops rather than throwing. + logger?.warn?.(line.slice(0, 4096)) } catch { // Best-effort debug logging — intentionally swallowed. }