Browse Source

refactor: Concept for alternative approach to telemetry

Michael Bromley 8 months ago
parent
commit
b42b34442a

+ 34 - 0
packages/core/src/common/instrument-decorator.ts

@@ -0,0 +1,34 @@
+import { getConfig } from '../config';
+
+export function Instrument(): ClassDecorator {
+    return function (target: Type<any>) {
+        return class extends (target as new (...args: any[]) => any) {
+            constructor(...args: any[]) {
+                // eslint-disable-next-line constructor-super
+                super(...args);
+                const config = getConfig();
+                const { instrumentationStrategy } = config.systemOptions;
+                if (!instrumentationStrategy) {
+                    return this;
+                }
+                return new Proxy(this, {
+                    get: (obj, prop) => {
+                        const original = obj[prop];
+                        if (typeof original === 'function') {
+                            return function (...methodArgs: any[]) {
+                                const wrappedMethodArgs = {
+                                    target,
+                                    methodName: String(prop),
+                                    args: methodArgs,
+                                    applyOriginalFunction: () => original.apply(obj, methodArgs),
+                                };
+                                return instrumentationStrategy.wrapMethod(wrappedMethodArgs);
+                            };
+                        }
+                        return original;
+                    },
+                });
+            }
+        };
+    };
+}

+ 1 - 0
packages/core/src/config/index.ts

@@ -91,6 +91,7 @@ export * from './shipping-method/shipping-eligibility-checker';
 export * from './shipping-method/shipping-line-assignment-strategy';
 export * from './system/error-handler-strategy';
 export * from './system/health-check-strategy';
+export * from './system/instrumentation-strategy';
 export * from './tax/address-based-tax-zone-strategy';
 export * from './tax/default-tax-line-calculation-strategy';
 export * from './tax/default-tax-zone-strategy';

+ 13 - 0
packages/core/src/config/system/instrumentation-strategy.ts

@@ -0,0 +1,13 @@
+import { Type } from '../../common/types/common-types';
+import { InjectableStrategy } from '../../common/types/injectable-strategy';
+
+export interface WrappedMethodArgs {
+    target: Type<any>;
+    methodName: string;
+    args: any[];
+    applyOriginalFunction: () => any | Promise<any>;
+}
+
+export interface InstrumentationStrategy extends InjectableStrategy {
+    wrapMethod(args: WrappedMethodArgs): any;
+}

+ 3 - 1
packages/core/src/config/vendure-config.ts

@@ -25,10 +25,10 @@ import { ProductVariantPriceUpdateStrategy } from './catalog/product-variant-pri
 import { StockDisplayStrategy } from './catalog/stock-display-strategy';
 import { StockLocationStrategy } from './catalog/stock-location-strategy';
 import { CustomFields } from './custom-field/custom-field-types';
+import { EntityMetadataModifier } from './entity-metadata/entity-metadata-modifier';
 import { EntityDuplicator } from './entity/entity-duplicator';
 import { EntityIdStrategy } from './entity/entity-id-strategy';
 import { MoneyStrategy } from './entity/money-strategy';
-import { EntityMetadataModifier } from './entity-metadata/entity-metadata-modifier';
 import { FulfillmentHandler } from './fulfillment/fulfillment-handler';
 import { FulfillmentProcess } from './fulfillment/fulfillment-process';
 import { JobQueueStrategy } from './job-queue/job-queue-strategy';
@@ -58,6 +58,7 @@ import { ShippingLineAssignmentStrategy } from './shipping-method/shipping-line-
 import { CacheStrategy } from './system/cache-strategy';
 import { ErrorHandlerStrategy } from './system/error-handler-strategy';
 import { HealthCheckStrategy } from './system/health-check-strategy';
+import { InstrumentationStrategy } from './system/instrumentation-strategy';
 import { TaxLineCalculationStrategy } from './tax/tax-line-calculation-strategy';
 import { TaxZoneStrategy } from './tax/tax-zone-strategy';
 
@@ -1088,6 +1089,7 @@ export interface SystemOptions {
      * @default InMemoryCacheStrategy
      */
     cacheStrategy?: CacheStrategy;
+    instrumentationStrategy?: InstrumentationStrategy | undefined;
 }
 
 /**

+ 2 - 15
packages/core/src/service/services/product.service.ts

@@ -19,6 +19,7 @@ import { RelationPaths } from '../../api/decorators/relations.decorator';
 import { ErrorResultUnion } from '../../common/error/error-result';
 import { EntityNotFoundError, InternalServerError, UserInputError } from '../../common/error/errors';
 import { ProductOptionInUseError } from '../../common/error/generated-graphql-admin-errors';
+import { Instrument } from '../../common/instrument-decorator';
 import { ListQueryOptions } from '../../common/types/common-types';
 import { Translated } from '../../common/types/locale-types';
 import { assertFound, idsAreEqual } from '../../common/utils';
@@ -33,7 +34,6 @@ import { EventBus } from '../../event-bus/event-bus';
 import { ProductChannelEvent } from '../../event-bus/events/product-channel-event';
 import { ProductEvent } from '../../event-bus/events/product-event';
 import { ProductOptionGroupChangeEvent } from '../../event-bus/events/product-option-group-change-event';
-import { Span } from '../../instrumentation';
 import { CustomFieldRelationService } from '../helpers/custom-field-relation/custom-field-relation.service';
 import { ListQueryBuilder } from '../helpers/list-query-builder/list-query-builder';
 import { SlugValidator } from '../helpers/slug-validator/slug-validator';
@@ -53,6 +53,7 @@ import { ProductVariantService } from './product-variant.service';
  * @docsCategory services
  */
 @Injectable()
+@Instrument()
 export class ProductService {
     private readonly relations = ['featuredAsset', 'assets', 'channels', 'facetValues', 'facetValues.facet'];
 
@@ -71,7 +72,6 @@ export class ProductService {
         private productOptionGroupService: ProductOptionGroupService,
     ) {}
 
-    @Span('ProductService.findAll')
     async findAll(
         ctx: RequestContext,
         options?: ListQueryOptions<Product>,
@@ -127,7 +127,6 @@ export class ProductService {
             });
     }
 
-    @Span('ProductService.findOne')
     async findOne(
         ctx: RequestContext,
         productId: ID,
@@ -157,7 +156,6 @@ export class ProductService {
         return this.translator.translate(product, ctx, ['facetValues', ['facetValues', 'facet']]);
     }
 
-    @Span('ProductService.findByIds')
     async findByIds(
         ctx: RequestContext,
         productIds: ID[],
@@ -191,7 +189,6 @@ export class ProductService {
      * @description
      * Returns all Channels to which the Product is assigned.
      */
-    @Span('ProductService.getProductChannels')
     async getProductChannels(ctx: RequestContext, productId: ID): Promise<Channel[]> {
         const span = getActiveSpan();
         span?.setAttribute('productId', productId.toString());
@@ -204,7 +201,6 @@ export class ProductService {
         return product.channels;
     }
 
-    @Span('ProductService.getFacetValuesForProduct')
     getFacetValuesForProduct(ctx: RequestContext, productId: ID): Promise<Array<Translated<FacetValue>>> {
         const span = getActiveSpan();
         span?.setAttribute('productId', productId.toString());
@@ -226,7 +222,6 @@ export class ProductService {
             });
     }
 
-    @Span('ProductService.findOneBySlug')
     async findOneBySlug(
         ctx: RequestContext,
         slug: string,
@@ -267,7 +262,6 @@ export class ProductService {
         }
     }
 
-    @Span('ProductService.create')
     async create(ctx: RequestContext, input: CreateProductInput): Promise<Translated<Product>> {
         const span = getActiveSpan();
         span?.setAttribute('productName', input.translations?.[0]?.name || 'unnamed');
@@ -294,7 +288,6 @@ export class ProductService {
         return assertFound(this.findOne(ctx, product.id));
     }
 
-    @Span('ProductService.update')
     async update(ctx: RequestContext, input: UpdateProductInput): Promise<Translated<Product>> {
         const span = getActiveSpan();
         span?.setAttribute('productId', input.id.toString());
@@ -329,7 +322,6 @@ export class ProductService {
         return assertFound(this.findOne(ctx, updatedProduct.id));
     }
 
-    @Span('ProductService.softDelete')
     async softDelete(ctx: RequestContext, productId: ID): Promise<DeletionResponse> {
         const span = getActiveSpan();
         span?.setAttribute('productId', productId.toString());
@@ -379,7 +371,6 @@ export class ProductService {
      * Internally, this method will also call {@link ProductVariantService} `assignProductVariantsToChannel()` for
      * each of the Product's variants, and will assign the Product's Assets to the Channel too.
      */
-    @Span('ProductService.assignProductsToChannel')
     async assignProductsToChannel(
         ctx: RequestContext,
         input: AssignProductsToChannelInput,
@@ -416,7 +407,6 @@ export class ProductService {
         );
     }
 
-    @Span('ProductService.removeProductsFromChannel')
     async removeProductsFromChannel(
         ctx: RequestContext,
         input: RemoveProductsFromChannelInput,
@@ -448,7 +438,6 @@ export class ProductService {
         );
     }
 
-    @Span('ProductService.addOptionGroupToProduct')
     async addOptionGroupToProduct(
         ctx: RequestContext,
         productId: ID,
@@ -487,7 +476,6 @@ export class ProductService {
         return assertFound(this.findOne(ctx, productId));
     }
 
-    @Span('ProductService.removeOptionGroupFromProduct')
     async removeOptionGroupFromProduct(
         ctx: RequestContext,
         productId: ID,
@@ -543,7 +531,6 @@ export class ProductService {
         return assertFound(this.findOne(ctx, productId));
     }
 
-    @Span('ProductService.getProductWithOptionGroups')
     private async getProductWithOptionGroups(ctx: RequestContext, productId: ID): Promise<Product> {
         const span = getActiveSpan();
         span?.setAttribute('productId', productId.toString());

+ 20 - 0
packages/dev-server/dev-config.ts

@@ -8,6 +8,8 @@ import {
     LanguageCode,
     OtelLogger,
     VendureConfig,
+    InstrumentationStrategy,
+    WrappedMethodArgs,
 } from '@vendure/core';
 import { defaultEmailHandlers, EmailPlugin, FileBasedTemplateLoader } from '@vendure/email-plugin';
 import { BullMQJobQueuePlugin } from '@vendure/job-queue-plugin/package/bullmq';
@@ -15,6 +17,20 @@ import 'dotenv/config';
 import path from 'path';
 import { DataSourceOptions } from 'typeorm';
 import { ReviewsPlugin } from './test-plugins/reviews/reviews-plugin';
+import { TelemetryPlugin } from '@vendure/telemetry';
+
+class TestInstrumentationStrategy implements InstrumentationStrategy {
+    wrapMethod({
+                   target,
+                   methodName,
+                   args,
+                   applyOriginalFunction,
+               }: WrappedMethodArgs) {
+        console.log(`Executing method: ${target.name}.${methodName}`);
+        applyOriginalFunction();
+        console.log(`Method ${target.name}.${methodName} completed`);
+    }
+}
 
 /**
  * Config settings used during development
@@ -55,6 +71,9 @@ export const devConfig: VendureConfig = {
     paymentOptions: {
         paymentMethodHandlers: [dummyPaymentHandler],
     },
+    // systemOptions: {
+    //   instrumentationStrategy: new TestInstrumentationStrategy(),
+    // },
     customFields: {
         Product: [
             {
@@ -92,6 +111,7 @@ export const devConfig: VendureConfig = {
         //     platformFeePercent: 10,
         //     platformFeeSKU: 'FEE',
         // }),
+        TelemetryPlugin,
         ReviewsPlugin,
         AssetServerPlugin.init({
             route: 'assets',

+ 1 - 1
packages/dev-server/instrumentation.ts

@@ -15,7 +15,7 @@ process.env.OTEL_LOGS_EXPORTER = 'otlp';
 
 const logExporter = new OTLPLogExporter();
 
-const config = getSdkConfiguration(true, {
+const config = getSdkConfiguration(false, {
     spanProcessors: [new BatchSpanProcessor(traceExporter)],
     logRecordProcessors: [new BatchLogRecordProcessor(logExporter)],
     resource: resourceFromAttributes({

+ 44 - 0
packages/telemetry/src/config/otel-instrumentation-strategy.ts

@@ -0,0 +1,44 @@
+import { Span as ApiSpan, SpanStatusCode, trace } from '@opentelemetry/api';
+import { logs } from '@opentelemetry/api-logs';
+import { InstrumentationStrategy, VENDURE_VERSION, WrappedMethodArgs } from '@vendure/core';
+
+export const tracer = trace.getTracer('@vendure/core', VENDURE_VERSION);
+export const otelLogger = logs.getLogger('@vendure/core', VENDURE_VERSION);
+const recordException = (span: ApiSpan, error: any) => {
+    span.recordException(error);
+    span.setStatus({ code: SpanStatusCode.ERROR, message: error.message });
+};
+
+export class OtelInstrumentationStrategy implements InstrumentationStrategy {
+    wrapMethod({ target, methodName, args, applyOriginalFunction }: WrappedMethodArgs) {
+        const spanName = `${String(target.name)}.${String(methodName)}`;
+
+        return tracer.startActiveSpan(spanName, {}, span => {
+            if (applyOriginalFunction.constructor.name === 'AsyncFunction') {
+                return (
+                    applyOriginalFunction()
+                        // @ts-expect-error
+                        .catch(error => {
+                            recordException(span, error);
+                            // Throw error to propagate it further
+                            throw error;
+                        })
+                        .finally(() => {
+                            span.end();
+                        })
+                );
+            }
+
+            try {
+                return applyOriginalFunction();
+            } catch (error) {
+                recordException(span, error);
+
+                // throw for further propagation
+                throw error;
+            } finally {
+                span.end();
+            }
+        });
+    }
+}

+ 1 - 0
packages/telemetry/src/index.ts

@@ -1,4 +1,5 @@
 export * from './decorator/span';
 export * from './instrumentation';
+export * from './telemetry.plugin';
 export * from './tracing/trace.service';
 export * from './utils/span';

+ 12 - 0
packages/telemetry/src/telemetry.plugin.ts

@@ -0,0 +1,12 @@
+import { PluginCommonModule, VendurePlugin } from '@vendure/core';
+
+import { OtelInstrumentationStrategy } from './config/otel-instrumentation-strategy';
+
+@VendurePlugin({
+    imports: [PluginCommonModule],
+    configuration: config => {
+        config.systemOptions.instrumentationStrategy = new OtelInstrumentationStrategy();
+        return config;
+    },
+})
+export class TelemetryPlugin {}