From 785bc3565294d3d3b13af9b8507eb87484f25bbd Mon Sep 17 00:00:00 2001 From: Brandon Krigbaum Date: Sun, 30 Aug 2026 11:24:20 -0500 Subject: [PATCH] Record how rapid identification matched, so adoption is measurable `matchedBy` shipped in v1.10.0 as the adoption observable and then went nowhere: clients select it and discard it, and nothing records it. The only signal was `noticeError`, which counts failures with no denominator -- you could see refusals but not the rate. Records three attributes on the transaction instead. One is not enough: `matchedBy: 'email'` conflates three populations that need completely different responses -- - the member has no stored id yet (client rollout, or a genuinely new member) - the member's ids are all dead, malformed, or unreachable - the member's ids point at two simultaneously-active customers The first resolves itself as the fleet adopts, the second is an infrastructure or data question, and the third is a duplicate pair that needs merging in Omeda. So `resolveOmedaCustomerId` now returns `{ customerId, outcome }` rather than a bare id, and the resolver reports: omedaMatchedBy customerId | email -- what Omeda actually matched on omedaIdResolution resolved | diverged | none-active | unresolvable | too-many | malformed-only | none-supplied omedaCandidateIdCount how many ids the caller sent -- the adoption denominator The reason is returned rather than only reported because distinguishing those cases by parsing `noticeError` message strings would be fragile. `resolved` alongside `matchedBy: 'email'` is meaningful on its own: that is the lookup-to-post race, where the id resolved cleanly and SCAO then rejected it. Additive and internal -- no schema change, no client change, so no coordinated deploy. The agent no-ops when disabled or outside a transaction, which matters on the blocking path of authentication. 14 specs (was 13): every outcome in the vocabulary is now asserted, including a new malformed-only case that nothing previously covered. Co-Authored-By: Claude Opus 5 --- services/graphql/src/resolvers/customer.js | 27 +++++- .../src/utils/resolve-omeda-customer-id.js | 39 ++++++-- .../utils/resolve-omeda-customer-id.spec.js | 97 ++++++++++++++----- 3 files changed, 127 insertions(+), 36 deletions(-) diff --git a/services/graphql/src/resolvers/customer.js b/services/graphql/src/resolvers/customer.js index 5fc0071..e18f2e8 100644 --- a/services/graphql/src/resolvers/customer.js +++ b/services/graphql/src/resolvers/customer.js @@ -427,9 +427,9 @@ module.exports = { } // Resolve the caller's stored encrypted ids (if any) to the one canonical, currently-active - // numeric customer id they agree on. Never throws: `null` means "fall back to email + // numeric customer id they agree on. Never throws: a null id means "fall back to email // matching", which is exactly the behaviour every caller had before this field existed. - const resolvedCustomerId = await resolveOmedaCustomerId({ + const { customerId: resolvedCustomerId, outcome } = await resolveOmedaCustomerId({ apiClient, encryptedCustomerIds, noticeError, @@ -564,6 +564,29 @@ module.exports = { ]); })(), ]); + /** + * Adoption telemetry. `matchedBy` is returned to the caller, but no client does anything with + * it -- so without this the only observable is `noticeError`, which counts *failures* with no + * denominator. Recording it here covers every caller and every hook path from one place. + * + * Three attributes, because one cannot answer the question. `omedaMatchedBy` alone conflates + * "the member had no stored id" with "the member's ids contradicted each other" -- the first + * is client rollout or a genuinely new member, the second is a duplicate pair needing an + * Omeda merge. `omedaIdResolution` separates them, and `omedaCandidateIdCount` gives the + * denominator for adoption. + * + * A `resolved` outcome alongside `matchedBy: 'email'` is meaningful on its own: it is the + * lookup-to-post race, where the id resolved cleanly and SCAO then rejected it. + * + * The agent no-ops when disabled or outside a transaction, so this is inert rather than + * fatal -- which matters on the blocking path of authentication. + */ + newrelic.addCustomAttributes({ + omedaMatchedBy: matchedBy, + omedaIdResolution: outcome, + omedaCandidateIdCount: getAsArray(input, 'encryptedCustomerIds').length, + }); + // `matchedBy` is resolver-provided, not an Omeda API value. Spreading preserves `CustomerId`, // which the `RapidCustomerIdentification.customer` field resolver destructures. return { ...response.data, matchedBy }; diff --git a/services/graphql/src/utils/resolve-omeda-customer-id.js b/services/graphql/src/utils/resolve-omeda-customer-id.js index b7f92bd..a5ff345 100644 --- a/services/graphql/src/utils/resolve-omeda-customer-id.js +++ b/services/graphql/src/utils/resolve-omeda-customer-id.js @@ -72,7 +72,12 @@ * @param {object} params.apiClient The Omeda API client. * @param {string[]} [params.encryptedCustomerIds] Candidate encrypted ids for this customer. * @param {function} params.noticeError Error reporter (New Relic's `noticeError`). - * @returns {Promise} The agreed numeric customer id, or `null` to fall back to email. + * @returns {Promise<{ customerId: ?number, outcome: string }>} The agreed numeric customer id (or + * `null` to fall back to email), plus *why* — see `OUTCOMES`. The reason is returned rather than + * only reported, because "no id was stored" and "ids were stored but contradicted each other" are + * the same `null` here and completely different problems: the first is client rollout or a + * brand-new member, the second is a duplicate pair that needs merging in Omeda. Counting + * `noticeError` message strings to tell them apart would be fragile. */ /** @@ -86,12 +91,27 @@ const MAX_CANDIDATES = 4; /** Omeda encrypted customer ids are exactly this long; see the api client's attribute schema. */ const ENCRYPTED_ID_LENGTH = 15; +/** + * Why a resolution ended where it did. Faceting these separates the three populations that all + * look like "matched by email" from the outside: the client has nothing stored yet, the member is + * genuinely new, or the member's stored ids point at contradictory records. + */ +const OUTCOMES = { + NONE_SUPPLIED: 'none-supplied', + MALFORMED_ONLY: 'malformed-only', + TOO_MANY: 'too-many', + UNRESOLVABLE: 'unresolvable', + NONE_ACTIVE: 'none-active', + DIVERGED: 'diverged', + RESOLVED: 'resolved', +}; + module.exports = async ({ apiClient, encryptedCustomerIds, noticeError } = {}) => { const supplied = [...new Set((encryptedCustomerIds || []) .filter((id) => id) .map((id) => `${id}`.trim()))]; // Not an error: most callers have no stored id yet. - if (!supplied.length) return null; + if (!supplied.length) return { customerId: null, outcome: OUTCOMES.NONE_SUPPLIED }; // Malformed values can never name a customer, so they carry no claim about a write target and // are dropped rather than allowed to veto a sibling. Mirrors the api client's own @@ -101,11 +121,11 @@ module.exports = async ({ apiClient, encryptedCustomerIds, noticeError } = {}) = if (candidates.length !== supplied.length) { noticeError(new Error(`Ignoring ${supplied.length - candidates.length} malformed encrypted customer id(s): ${supplied.filter((id) => id.length !== ENCRYPTED_ID_LENGTH).join(', ')}.`)); } - if (!candidates.length) return null; + if (!candidates.length) return { customerId: null, outcome: OUTCOMES.MALFORMED_ONLY }; if (candidates.length > MAX_CANDIDATES) { noticeError(new Error(`Refusing to resolve an Omeda customer from ${candidates.length} candidate encrypted ids (max ${MAX_CANDIDATES}). Falling back to email matching.`)); - return null; + return { customerId: null, outcome: OUTCOMES.TOO_MANY }; } const resource = apiClient.resource('customer'); @@ -130,7 +150,7 @@ module.exports = async ({ apiClient, encryptedCustomerIds, noticeError } = {}) = // Refuse rather than let a sibling's answer stand in for it. if (settled.some(({ state }) => state === 'unknown')) { noticeError(new Error(`Could not resolve every candidate encrypted id (${candidates.join(', ')}); cannot rule out a second active customer. Falling back to email matching.`)); - return null; + return { customerId: null, outcome: OUTCOMES.UNRESOLVABLE }; } // Conclusively-dead ids are ignored, not disqualifying: a merged-away or unknown id alongside a @@ -139,7 +159,7 @@ module.exports = async ({ apiClient, encryptedCustomerIds, noticeError } = {}) = if (!resolved.length) { noticeError(new Error(`Unable to resolve an Omeda customer from ${candidates.length} encrypted id(s): none are active. Falling back to email matching.`)); - return null; + return { customerId: null, outcome: OUTCOMES.NONE_ACTIVE }; } if (resolved.length > 1) { @@ -147,8 +167,11 @@ module.exports = async ({ apiClient, encryptedCustomerIds, noticeError } = {}) = // record receives every future write, so refuse -- email matching continues as it does today. // These are the pairs that need merging in Omeda; this is the signal that says which. noticeError(new Error(`Omeda customer ids ${resolved.join(', ')} are all active for the same member (encrypted ids ${candidates.join(', ')}); cannot choose a write target. Falling back to email matching.`)); - return null; + return { customerId: null, outcome: OUTCOMES.DIVERGED }; } - return resolved[0]; + return { customerId: resolved[0], outcome: OUTCOMES.RESOLVED }; }; + +// Attached after the function assignment above, which would otherwise clobber it. +module.exports.OUTCOMES = OUTCOMES; diff --git a/services/graphql/test/utils/resolve-omeda-customer-id.spec.js b/services/graphql/test/utils/resolve-omeda-customer-id.spec.js index 5fcd15d..ef3a07f 100644 --- a/services/graphql/test/utils/resolve-omeda-customer-id.spec.js +++ b/services/graphql/test/utils/resolve-omeda-customer-id.spec.js @@ -2,6 +2,8 @@ const { describe, it } = require('mocha'); const { expect } = require('chai'); const resolveOmedaCustomerId = require('../../src/utils/resolve-omeda-customer-id'); +const { OUTCOMES } = resolveOmedaCustomerId; + const LIVE = '9130C2719701F5S'; const MERGED = '6466A3060334H6A'; const DEAD = '0240G4865912F6U'; @@ -41,13 +43,15 @@ describe('utils/resolve-omeda-customer-id', () => { const { apiClient, calls } = clientWith({ [LIVE]: 1105483508 }); const { errors, noticeError } = noticer(); - const id = await resolveOmedaCustomerId({ + const { customerId, outcome } = await resolveOmedaCustomerId({ apiClient, encryptedCustomerIds: [LIVE], noticeError, }); - expect(id).to.equal(1105483508); + expect(customerId).to.equal(1105483508); + + expect(outcome).to.equal(OUTCOMES.RESOLVED); expect(errors).to.have.lengthOf(0); // Merge-following must be requested, and a miss must not throw -- both are load-bearing. expect(calls).to.deep.equal([{ @@ -63,13 +67,15 @@ describe('utils/resolve-omeda-customer-id', () => { const { apiClient } = clientWith({ [MERGED]: 1100158437 }); const { errors, noticeError } = noticer(); - const id = await resolveOmedaCustomerId({ + const { customerId, outcome } = await resolveOmedaCustomerId({ apiClient, encryptedCustomerIds: [MERGED], noticeError, }); - expect(id).to.equal(1100158437); + expect(customerId).to.equal(1100158437); + + expect(outcome).to.equal(OUTCOMES.RESOLVED); expect(errors).to.have.lengthOf(0); }); @@ -81,13 +87,15 @@ describe('utils/resolve-omeda-customer-id', () => { const { apiClient, calls } = clientWith({ [MERGED]: 1100158437, [LIVE]: 1100158437 }); const { errors, noticeError } = noticer(); - const id = await resolveOmedaCustomerId({ + const { customerId, outcome } = await resolveOmedaCustomerId({ apiClient, encryptedCustomerIds: [MERGED, LIVE], noticeError, }); - expect(id).to.equal(1100158437); + expect(customerId).to.equal(1100158437); + + expect(outcome).to.equal(OUTCOMES.RESOLVED); expect(calls).to.have.lengthOf(2); expect(errors).to.have.lengthOf(0); }); @@ -97,13 +105,15 @@ describe('utils/resolve-omeda-customer-id', () => { const { apiClient } = clientWith({ [DEAD]: null, [LIVE]: 1105483508 }); const { errors, noticeError } = noticer(); - const id = await resolveOmedaCustomerId({ + const { customerId, outcome } = await resolveOmedaCustomerId({ apiClient, encryptedCustomerIds: [DEAD, LIVE], noticeError, }); - expect(id).to.equal(1105483508); + expect(customerId).to.equal(1105483508); + + expect(outcome).to.equal(OUTCOMES.RESOLVED); expect(errors).to.have.lengthOf(0); }); @@ -117,13 +127,15 @@ describe('utils/resolve-omeda-customer-id', () => { }); const { errors, noticeError } = noticer(); - const id = await resolveOmedaCustomerId({ + const { customerId, outcome } = await resolveOmedaCustomerId({ apiClient, encryptedCustomerIds: [LIVE, MERGED, DEAD], noticeError, }); - expect(id).to.equal(1105483508); + expect(customerId).to.equal(1105483508); + + expect(outcome).to.equal(OUTCOMES.RESOLVED); expect(errors).to.have.lengthOf(0); }); @@ -133,13 +145,15 @@ describe('utils/resolve-omeda-customer-id', () => { const { apiClient, calls } = clientWith({ [LIVE]: 1105483508 }); const { errors, noticeError } = noticer(); - const id = await resolveOmedaCustomerId({ + const { customerId, outcome } = await resolveOmedaCustomerId({ apiClient, encryptedCustomerIds: ['too-short', LIVE], noticeError, }); - expect(id).to.equal(1105483508); + expect(customerId).to.equal(1105483508); + + expect(outcome).to.equal(OUTCOMES.RESOLVED); expect(calls.map((c) => c.encryptedId)).to.deep.equal([LIVE]); expect(errors).to.have.lengthOf(1); expect(errors[0].message).to.contain('malformed'); @@ -155,13 +169,15 @@ describe('utils/resolve-omeda-customer-id', () => { }); const { errors, noticeError } = noticer(); - const id = await resolveOmedaCustomerId({ + const { customerId, outcome } = await resolveOmedaCustomerId({ apiClient, encryptedCustomerIds: [LIVE, OTHER], noticeError, }); - expect(id).to.equal(null); + expect(customerId).to.equal(null); + + expect(outcome).to.equal(OUTCOMES.UNRESOLVABLE); expect(errors.map((e) => e.message).join(' ')).to.contain('socket hang up'); expect(errors.map((e) => e.message).join(' ')).to.contain('cannot rule out a second active customer'); }); @@ -170,13 +186,15 @@ describe('utils/resolve-omeda-customer-id', () => { const { apiClient } = clientWith({ [LIVE]: new Error('socket hang up') }); const { errors, noticeError } = noticer(); - const id = await resolveOmedaCustomerId({ + const { customerId, outcome } = await resolveOmedaCustomerId({ apiClient, encryptedCustomerIds: [LIVE], noticeError, }); - expect(id).to.equal(null); + expect(customerId).to.equal(null); + + expect(outcome).to.equal(OUTCOMES.UNRESOLVABLE); expect(errors).to.have.lengthOf(2); }); @@ -187,13 +205,15 @@ describe('utils/resolve-omeda-customer-id', () => { const { apiClient } = clientWith({ [LIVE]: 1105483508, [OTHER]: 1108476082 }); const { errors, noticeError } = noticer(); - const id = await resolveOmedaCustomerId({ + const { customerId, outcome } = await resolveOmedaCustomerId({ apiClient, encryptedCustomerIds: [LIVE, OTHER], noticeError, }); - expect(id).to.equal(null); + expect(customerId).to.equal(null); + + expect(outcome).to.equal(OUTCOMES.DIVERGED); expect(errors).to.have.lengthOf(1); expect(errors[0].message).to.contain('are all active for the same member'); // The report must name the customers, so the pairs needing an Omeda merge are identifiable. @@ -205,13 +225,15 @@ describe('utils/resolve-omeda-customer-id', () => { const { apiClient } = clientWith({ [DEAD]: null, [OTHER]: null }); const { errors, noticeError } = noticer(); - const id = await resolveOmedaCustomerId({ + const { customerId, outcome } = await resolveOmedaCustomerId({ apiClient, encryptedCustomerIds: [DEAD, OTHER], noticeError, }); - expect(id).to.equal(null); + expect(customerId).to.equal(null); + + expect(outcome).to.equal(OUTCOMES.NONE_ACTIVE); expect(errors).to.have.lengthOf(1); expect(errors[0].message).to.contain('none are active'); }); @@ -220,13 +242,15 @@ describe('utils/resolve-omeda-customer-id', () => { const { apiClient, calls } = clientWith({ [LIVE]: 1105483508 }); const { errors, noticeError } = noticer(); - const id = await resolveOmedaCustomerId({ + const { customerId, outcome } = await resolveOmedaCustomerId({ apiClient, encryptedCustomerIds: [LIVE, LIVE, LIVE], noticeError, }); - expect(id).to.equal(1105483508); + expect(customerId).to.equal(1105483508); + + expect(outcome).to.equal(OUTCOMES.RESOLVED); expect(calls).to.have.lengthOf(1); expect(errors).to.have.lengthOf(0); }); @@ -236,27 +260,48 @@ describe('utils/resolve-omeda-customer-id', () => { const { apiClient, calls } = clientWith({}); const { errors, noticeError } = noticer(); - const id = await resolveOmedaCustomerId({ + const { customerId, outcome } = await resolveOmedaCustomerId({ apiClient, encryptedCustomerIds: [LIVE, MERGED, DEAD, OTHER, '1234A5678901B2C'], noticeError, }); - expect(id).to.equal(null); + expect(customerId).to.equal(null); + + expect(outcome).to.equal(OUTCOMES.TOO_MANY); expect(calls).to.have.lengthOf(0); expect(errors).to.have.lengthOf(1); expect(errors[0].message).to.contain('max 4'); }); + it('reports malformed-only when every candidate is unusable', async () => { + // Distinct from `none-supplied`: the client DID store something, it just cannot name a + // customer. Faceting these apart is the point of returning an outcome at all. + const { apiClient, calls } = clientWith({}); + const { errors, noticeError } = noticer(); + + const { customerId, outcome } = await resolveOmedaCustomerId({ + apiClient, + encryptedCustomerIds: ['too-short', 'also-bad'], + noticeError, + }); + + expect(customerId).to.equal(null); + expect(outcome).to.equal(OUTCOMES.MALFORMED_ONLY); + expect(calls).to.have.lengthOf(0); + expect(errors).to.have.lengthOf(1); + }); + it('returns null without an API call or an error report when no ids are supplied', async () => { const { apiClient, calls } = clientWith({}); const { errors, noticeError } = noticer(); - const ids = await Promise.all([undefined, null, [], ['', null]].map((encryptedCustomerIds) => ( + const results = await Promise.all([undefined, null, [], ['', null]].map((encryptedCustomerIds) => ( resolveOmedaCustomerId({ apiClient, encryptedCustomerIds, noticeError }) ))); - expect(ids).to.deep.equal([null, null, null, null]); + expect(results.map((r) => r.customerId)).to.deep.equal([null, null, null, null]); + expect(results.map((r) => r.outcome)).to.deep.equal(Array(4).fill(OUTCOMES.NONE_SUPPLIED)); expect(calls).to.have.lengthOf(0); // Holding no stored id is the common case, not a failure -- it must not create error noise. expect(errors).to.have.lengthOf(0);