diff --git a/CHANGELOG.md b/CHANGELOG.md index b59926a85c..82779521e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ the release. ## Unreleased +* [payment] Fix `charge` span lifecycle and exception attribution: wrap charge + logic in `try/catch/finally` to ensure the span is always ended, record + exceptions on the `charge` span where they originate, and remove duplicate + `recordException` from the gRPC handler + ([#3276](https://github.com/open-telemetry/opentelemetry-demo/pull/3276)) +* [recommendation] Fix `recommendationCacheFailure` feature flag by + using `ListProducts` instead of `GetProduct` + ([#3260](https://github.com/open-telemetry/opentelemetry-demo/pull/3260)) * [telemetry-docs] Add a new service to provide telemetry documentation based on Weaver ([#2794](https://github.com/open-telemetry/opentelemetry-demo/pull/2794)) diff --git a/src/payment/charge.js b/src/payment/charge.js index 588cc0754e..cdf684241b 100644 --- a/src/payment/charge.js +++ b/src/payment/charge.js @@ -1,6 +1,6 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -const { context, propagation, trace, metrics } = require('@opentelemetry/api'); +const { context, propagation, trace, metrics, SpanStatusCode } = require('@opentelemetry/api'); const cardValidator = require('simple-card-validator'); const { v4: uuidv4 } = require('uuid'); @@ -24,65 +24,72 @@ function random(arr) { module.exports.charge = async request => { const span = tracer.startSpan('charge'); - await OpenFeature.setProviderAndWait(flagProvider); + try { + await OpenFeature.setProviderAndWait(flagProvider); - const numberVariant = await OpenFeature.getClient().getNumberValue("paymentFailure", 0); + const numberVariant = await OpenFeature.getClient().getNumberValue("paymentFailure", 0); - if (numberVariant > 0) { - // n% chance to fail with app.loyalty.level=gold - if (Math.random() < numberVariant) { - span.setAttributes({'app.loyalty.level': 'gold' }); - span.end(); + if (numberVariant > 0) { + // n% chance to fail with app.loyalty.level=gold + if (Math.random() < numberVariant) { + span.setAttributes({'app.loyalty.level': 'gold' }); - throw new Error('Payment request failed. Invalid token. app.loyalty.level=gold'); + throw new Error('Payment request failed. Invalid token. app.loyalty.level=gold'); + } } - } - const { - creditCardNumber: number, - creditCardExpirationYear: year, - creditCardExpirationMonth: month - } = request.creditCard; - const currentMonth = new Date().getMonth() + 1; - const currentYear = new Date().getFullYear(); - const lastFourDigits = number.substr(-4); - const transactionId = uuidv4(); - - const card = cardValidator(number); - const { card_type: cardType, valid } = card.getCardDetails(); - - const loyalty_level = random(LOYALTY_LEVEL); - - span.setAttributes({ - 'app.payment.card_type': cardType, - 'app.payment.card_valid': valid, - 'app.loyalty.level': loyalty_level - }); - - if (!valid) { - throw new Error('Credit card info is invalid.'); - } + const { + creditCardNumber: number, + creditCardExpirationYear: year, + creditCardExpirationMonth: month + } = request.creditCard; + const currentMonth = new Date().getMonth() + 1; + const currentYear = new Date().getFullYear(); + const lastFourDigits = number.substr(-4); + const transactionId = uuidv4(); + + const card = cardValidator(number); + const { card_type: cardType, valid } = card.getCardDetails(); + + const loyalty_level = random(LOYALTY_LEVEL); + + span.setAttributes({ + 'app.payment.card_type': cardType, + 'app.payment.card_valid': valid, + 'app.loyalty.level': loyalty_level + }); + + if (!valid) { + throw new Error('Credit card info is invalid.'); + } - if (!['visa', 'mastercard'].includes(cardType)) { - throw new Error(`Sorry, we cannot process ${cardType} credit cards. Only VISA or MasterCard is accepted.`); - } + if (!['visa', 'mastercard'].includes(cardType)) { + throw new Error(`Sorry, we cannot process ${cardType} credit cards. Only VISA or MasterCard is accepted.`); + } - if ((currentYear * 12 + currentMonth) > (year * 12 + month)) { - throw new Error(`The credit card (ending ${lastFourDigits}) expired on ${month}/${year}.`); - } + if ((currentYear * 12 + currentMonth) > (year * 12 + month)) { + throw new Error(`The credit card (ending ${lastFourDigits}) expired on ${month}/${year}.`); + } - // Check baggage for synthetic_request=true, and add charged attribute accordingly - const baggage = propagation.getBaggage(context.active()); - if (baggage && baggage.getEntry('synthetic_request') && baggage.getEntry('synthetic_request').value === 'true') { - span.setAttribute('app.payment.charged', false); - } else { - span.setAttribute('app.payment.charged', true); - } + // Check baggage for synthetic_request=true, and add charged attribute accordingly + const baggage = propagation.getBaggage(context.active()); + if (baggage && baggage.getEntry('synthetic_request') && baggage.getEntry('synthetic_request').value === 'true') { + span.setAttribute('app.payment.charged', false); + } else { + span.setAttribute('app.payment.charged', true); + } - const { units, nanos, currencyCode } = request.amount; - logger.info({ transactionId, cardType, lastFourDigits, amount: { units, nanos, currencyCode }, loyalty_level }, 'Transaction complete.'); - transactionsCounter.add(1, { 'app.payment.currency': currencyCode }); - span.end(); + const { units, nanos, currencyCode } = request.amount; + logger.info({ transactionId, cardType, lastFourDigits, amount: { units, nanos, currencyCode }, loyalty_level }, 'Transaction complete.'); + transactionsCounter.add(1, { 'app.payment.currency': currencyCode }); - return { transactionId }; + return { transactionId }; + } catch (err) { + span.recordException(err); + span.setStatus({ code: SpanStatusCode.ERROR, message: err.message }); + + throw err; + } finally { + span.end(); + } }; diff --git a/src/payment/index.js b/src/payment/index.js index 14c05e3d58..c3e3ae9c15 100644 --- a/src/payment/index.js +++ b/src/payment/index.js @@ -24,8 +24,7 @@ async function chargeServiceHandler(call, callback) { } catch (err) { logger.warn({ err }) - span?.recordException(err) - span?.setStatus({ code: opentelemetry.SpanStatusCode.ERROR }) + span?.setStatus({ code: opentelemetry.SpanStatusCode.ERROR, message: err.message }) callback(err) } } diff --git a/test/tracetesting/payment/amex-credit-card-not-allowed.yaml b/test/tracetesting/payment/amex-credit-card-not-allowed.yaml index 6ad88e7885..8593247d01 100644 --- a/test/tracetesting/payment/amex-credit-card-not-allowed.yaml +++ b/test/tracetesting/payment/amex-credit-card-not-allowed.yaml @@ -35,3 +35,8 @@ spec: selector: span[tracetest.span.type="general" name="Tracetest trigger"] assertions: - attr:tracetest.response.status = 2 + - name: It recorded an ERROR status on the charge span + selector: span[tracetest.span.type="general" name="charge"] + assertions: + - attr:tracetest.span.status_description = "Sorry, we cannot process amex credit cards. Only VISA or MasterCard is accepted." + - attr:tracetest.span.status_code = "STATUS_CODE_ERROR" diff --git a/test/tracetesting/payment/expired-credit-card.yaml b/test/tracetesting/payment/expired-credit-card.yaml index c2f9d5d020..6e87aa38ef 100644 --- a/test/tracetesting/payment/expired-credit-card.yaml +++ b/test/tracetesting/payment/expired-credit-card.yaml @@ -35,3 +35,8 @@ spec: selector: span[tracetest.span.type="general" name="Tracetest trigger"] assertions: - attr:tracetest.response.status = 2 + - name: It recorded an ERROR status on the charge span + selector: span[tracetest.span.type="general" name="charge"] + assertions: + - attr:tracetest.span.status_description = "The credit card (ending 0454) expired on 1/2021." + - attr:tracetest.span.status_code = "STATUS_CODE_ERROR" diff --git a/test/tracetesting/payment/invalid-credit-card.yaml b/test/tracetesting/payment/invalid-credit-card.yaml index 6ab64de72a..9b93550db2 100644 --- a/test/tracetesting/payment/invalid-credit-card.yaml +++ b/test/tracetesting/payment/invalid-credit-card.yaml @@ -35,3 +35,8 @@ spec: selector: span[tracetest.span.type="general" name="Tracetest trigger"] assertions: - attr:tracetest.response.status = 2 + - name: It recorded an ERROR status on the charge span + selector: span[tracetest.span.type="general" name="charge"] + assertions: + - attr:tracetest.span.status_description = "Credit card info is invalid." + - attr:tracetest.span.status_code = "STATUS_CODE_ERROR"