From 721a44a6d2e2b373c2021d3f1075699298ff4348 Mon Sep 17 00:00:00 2001 From: Evan Sims Date: Fri, 30 Aug 2024 18:10:37 -0500 Subject: [PATCH 01/30] feat: enhancements to OpenTelemetry support --- api.ts | 84 ++++++++++----------- common.ts | 66 +++++++++++------ configuration.ts | 10 +++ credentials/credentials.ts | 31 +++++--- index.ts | 5 ++ package-lock.json | 14 ---- package.json | 1 - telemetry.ts | 71 ------------------ telemetry/attributes.ts | 103 ++++++++++++++++++++++++++ telemetry/configuration.ts | 53 +++++++++++++ telemetry/counters.ts | 13 ++++ telemetry/histograms.ts | 19 +++++ telemetry/metrics.ts | 59 +++++++++++++++ tests/telemetry/attributes.test.ts | 69 +++++++++++++++++ tests/telemetry/configuration.test.ts | 68 +++++++++++++++++ tests/telemetry/counters.test.ts | 9 +++ tests/telemetry/histograms.test.ts | 15 ++++ tests/telemetry/metrics.test.ts | 42 +++++++++++ 18 files changed, 573 insertions(+), 159 deletions(-) delete mode 100644 telemetry.ts create mode 100644 telemetry/attributes.ts create mode 100644 telemetry/configuration.ts create mode 100644 telemetry/counters.ts create mode 100644 telemetry/histograms.ts create mode 100644 telemetry/metrics.ts create mode 100644 tests/telemetry/attributes.test.ts create mode 100644 tests/telemetry/configuration.test.ts create mode 100644 tests/telemetry/counters.test.ts create mode 100644 tests/telemetry/histograms.test.ts create mode 100644 tests/telemetry/metrics.test.ts diff --git a/api.ts b/api.ts index 381ab64..189841a 100644 --- a/api.ts +++ b/api.ts @@ -24,7 +24,6 @@ import { CallResult, PromiseResult } from "./common"; -import { attributeNames } from "./telemetry"; import { Configuration } from "./configuration"; import { Credentials } from "./credentials"; import { assertParamExists } from "./validation"; @@ -110,6 +109,7 @@ import { WriteRequestDeletes, WriteRequestWrites, } from "./apiModel"; +import { TelemetryAttributes } from "./telemetry/attributes"; /** @@ -759,10 +759,10 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async check(storeId: string, body: CheckRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.check(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "check", - [attributeNames.requestStoreId]: storeId, - [attributeNames.requestModelId]: body.authorization_model_id, - [attributeNames.user]: body.tuple_key.user + [TelemetryAttributes.fgaClientRequestMethod.name]: "Check", + [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId ?? "", + [TelemetryAttributes.fgaClientRequestModelId.name]: body.authorization_model_id ?? "", + [TelemetryAttributes.fgaClientUser.name]: body.tuple_key.user }); }, /** @@ -775,7 +775,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async createStore(body: CreateStoreRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.createStore(body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "createStore", + [TelemetryAttributes.fgaClientRequestMethod.name]: "createStore", }); }, /** @@ -788,8 +788,8 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async deleteStore(storeId: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.deleteStore(storeId, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "deleteStore", - [attributeNames.requestStoreId]: storeId, + [TelemetryAttributes.fgaClientRequestMethod.name]: "deleteStore", + [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, }); }, /** @@ -803,9 +803,9 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async expand(storeId: string, body: ExpandRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.expand(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "expand", - [attributeNames.requestModelId]: body.authorization_model_id, - [attributeNames.requestStoreId]: storeId, + [TelemetryAttributes.fgaClientRequestMethod.name]: "expand", + [TelemetryAttributes.fgaClientRequestModelId.name]: body.authorization_model_id ?? "", + [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId ?? "", }); }, /** @@ -818,8 +818,8 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async getStore(storeId: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.getStore(storeId, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "getStore", - [attributeNames.requestStoreId]: storeId, + [TelemetryAttributes.fgaClientRequestMethod.name]: "getStore", + [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, }); }, /** @@ -833,14 +833,14 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async listObjects(storeId: string, body: ListObjectsRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.listObjects(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "listObjects", - [attributeNames.requestStoreId]: storeId, - [attributeNames.requestModelId]: body.authorization_model_id, - [attributeNames.user]: body.user + [TelemetryAttributes.fgaClientRequestMethod.name]: "listObjects", + [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId ?? "", + [TelemetryAttributes.fgaClientRequestModelId.name]: body.authorization_model_id ?? "", + [TelemetryAttributes.fgaClientUser.name]: body.user }); }, /** - * Returns a paginated list of OpenFGA stores and a continuation token to get additional stores. The continuation token will be empty if there are no more stores. + * Returns a paginated list of OpenFGA stores and a continuation token to get additional stores. The continuation token will be empty if there are no more stores. * @summary List all stores * @param {number} [pageSize] * @param {string} [continuationToken] @@ -850,7 +850,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async listStores(pageSize?: number, continuationToken?: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.listStores(pageSize, continuationToken, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "listStores", + [TelemetryAttributes.fgaClientRequestMethod.name]: "listStores", }); }, /** @@ -864,9 +864,9 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async listUsers(storeId: string, body: ListUsersRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.listUsers(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "listUsers", - [attributeNames.requestStoreId]: storeId, - [attributeNames.requestModelId]: body.authorization_model_id, + [TelemetryAttributes.fgaClientRequestMethod.name]: "listUsers", + [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId ?? "", + [TelemetryAttributes.fgaClientRequestModelId.name]: body.authorization_model_id ?? "", }); }, /** @@ -880,12 +880,12 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async read(storeId: string, body: ReadRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.read(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "read", - [attributeNames.requestStoreId]: storeId, + [TelemetryAttributes.fgaClientRequestMethod.name]: "read", + [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, }); }, /** - * The ReadAssertions API will return, for a given authorization model id, all the assertions stored for it. An assertion is an object that contains a tuple key, and the expectation of whether a call to the Check API of that tuple key will return true or false. + * The ReadAssertions API will return, for a given authorization model id, all the assertions stored for it. An assertion is an object that contains a tuple key, and the expectation of whether a call to the Check API of that tuple key will return true or false. * @summary Read assertions for an authorization model ID * @param {string} storeId * @param {string} authorizationModelId @@ -895,9 +895,9 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async readAssertions(storeId: string, authorizationModelId: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.readAssertions(storeId, authorizationModelId, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "readAssertions", - [attributeNames.requestStoreId]: storeId, - [attributeNames.requestModelId]: authorizationModelId, + [TelemetryAttributes.fgaClientRequestMethod.name]: "readAssertions", + [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, + [TelemetryAttributes.fgaClientRequestModelId.name]: authorizationModelId, }); }, /** @@ -911,8 +911,8 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async readAuthorizationModel(storeId: string, id: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.readAuthorizationModel(storeId, id, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "readAuthorizationModel", - [attributeNames.requestStoreId]: storeId, + [TelemetryAttributes.fgaClientRequestMethod.name]: "readAuthorizationModel", + [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, }); }, /** @@ -927,8 +927,8 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async readAuthorizationModels(storeId: string, pageSize?: number, continuationToken?: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.readAuthorizationModels(storeId, pageSize, continuationToken, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "readAuthorizationModels", - [attributeNames.requestStoreId]: storeId, + [TelemetryAttributes.fgaClientRequestMethod.name]: "readAuthorizationModels", + [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, }); }, /** @@ -944,8 +944,8 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async readChanges(storeId: string, type?: string, pageSize?: number, continuationToken?: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.readChanges(storeId, type, pageSize, continuationToken, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "readChanges", - [attributeNames.requestStoreId]: storeId, + [TelemetryAttributes.fgaClientRequestMethod.name]: "readChanges", + [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, }); }, /** @@ -959,9 +959,9 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async write(storeId: string, body: WriteRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.write(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "write", - [attributeNames.requestStoreId]: storeId, - [attributeNames.requestModelId]: body.authorization_model_id, + [TelemetryAttributes.fgaClientRequestMethod.name]: "write", + [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId ?? "", + [TelemetryAttributes.fgaClientRequestModelId.name]: body.authorization_model_id ?? "", }); }, /** @@ -976,9 +976,9 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async writeAssertions(storeId: string, authorizationModelId: string, body: WriteAssertionsRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.writeAssertions(storeId, authorizationModelId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "writeAssertions", - [attributeNames.requestStoreId]: storeId, - [attributeNames.requestModelId]: authorizationModelId, + [TelemetryAttributes.fgaClientRequestMethod.name]: "writeAssertions", + [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, + [TelemetryAttributes.fgaClientRequestModelId.name]: authorizationModelId, }); }, /** @@ -992,8 +992,8 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async writeAuthorizationModel(storeId: string, body: WriteAuthorizationModelRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.writeAuthorizationModel(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [attributeNames.requestMethod]: "writeAuthorizationModel", - [attributeNames.requestStoreId]: storeId, + [TelemetryAttributes.fgaClientRequestMethod.name]: "writeAuthorizationModel", + [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, }); }, }; diff --git a/common.ts b/common.ts index f3415e2..6b71eca 100644 --- a/common.ts +++ b/common.ts @@ -12,7 +12,6 @@ import { AxiosInstance, AxiosRequestConfig, AxiosResponse } from "axios"; -import { metrics } from "@opentelemetry/api"; import { Configuration } from "./configuration"; import type { Credentials } from "./credentials"; @@ -26,17 +25,9 @@ import { FgaError } from "./errors"; import { setNotEnumerableProperty } from "./utils"; -import { buildAttributes } from "./telemetry"; - -const meter = metrics.getMeter("@openfga/sdk", "0.6.3"); -const durationHist = meter.createHistogram("fga-client.request.duration", { - description: "The duration of requests", - unit: "milliseconds", -}); -const queryDurationHist = meter.createHistogram("fga-client.query.duration", { - description: "The duration of queries on the FGA server", - unit: "milliseconds", -}); +import { TelemetryAttributes } from "./telemetry/attributes"; +import { TelemetryMetrics } from "./telemetry/metrics"; +import { TelemetryHistograms } from "./telemetry/histograms"; /** * @@ -127,6 +118,10 @@ export const toPathString = function (url: URL) { type ObjectOrVoid = object | void; +interface StringIndexable { + [key: string]: any; +} + export type CallResult = T & { $response: AxiosResponse }; @@ -192,7 +187,7 @@ export async function attemptHttpRequest( /** * creates an axios request function */ -export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInstance: AxiosInstance, configuration: Configuration, credentials: Credentials, methodAttributes: Record = {}) { +export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInstance: AxiosInstance, configuration: Configuration, credentials: Credentials, methodAttributes: Record = {}) { configuration.isValid(); const retryParams = axiosArgs.options?.retryParams ? axiosArgs.options?.retryParams : configuration.retryParams; @@ -209,22 +204,49 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst maxRetry, minWaitInMs, }, axios); - const executionTime = Date.now() - start; const data = typeof response?.data === "undefined" ? {} : response?.data; const result: CallResult = { ...data }; setNotEnumerableProperty(result, "$response", response); - const attributes = buildAttributes(response, configuration.credentials, methodAttributes); - - if (response?.headers) { - const duration = response.headers["fga-query-duration-ms"]; - if (duration !== undefined) { - queryDurationHist.record(parseInt(duration, 10), attributes); - } + const telemetryAttributes = new TelemetryAttributes(); + const telemetryMetrics = new TelemetryMetrics(); + + let attributes: StringIndexable = {}; + + attributes = telemetryAttributes.fromRequest({ + start: start, + credentials: credentials, + attributes: methodAttributes, + }); + + attributes = telemetryAttributes.fromResponse({ + response, + credentials, + attributes, + }); + + if (attributes[TelemetryAttributes.httpServerRequestDuration.name]) { + telemetryMetrics.histogram( + TelemetryHistograms.queryDuration, + parseInt(attributes[TelemetryAttributes.httpServerRequestDuration.name] as string, 10), + telemetryAttributes.prepare( + attributes, + Object.keys(configuration.telemetryConfig.metrics.histogramQueryDuration.attributes()) + ) + ); } - durationHist.record(executionTime, attributes); + if (attributes[TelemetryAttributes.httpClientRequestDuration.name]) { + telemetryMetrics.histogram( + TelemetryHistograms.requestDuration, + Date.now() - start, + telemetryAttributes.prepare( + attributes, + Object.keys(configuration.telemetryConfig.metrics.histogramRequestDuration.attributes()) + ) + ); + } return result; }; diff --git a/configuration.ts b/configuration.ts index 1c844bc..6e70aaa 100644 --- a/configuration.ts +++ b/configuration.ts @@ -14,6 +14,7 @@ import { ApiTokenConfig, AuthCredentialsConfig, ClientCredentialsConfig, CredentialsMethod } from "./credentials/types"; import { FgaValidationError, } from "./errors"; import { assertParamExists, isWellFormedUlidString, isWellFormedUriString } from "./validation"; +import { TelemetryConfiguration } from './telemetry/configuration'; // default maximum number of retry const DEFAULT_MAX_RETRY = 15; @@ -41,6 +42,7 @@ export interface UserConfigurationParams { credentials?: CredentialsConfig; baseOptions?: any; retryParams?: RetryParams; + telemetryConfig?: TelemetryConfiguration; } export function GetDefaultRetryParams (maxRetry = DEFAULT_MAX_RETRY, minWaitInMs = DEFAULT_MIN_WAIT_MS) { @@ -120,6 +122,13 @@ export class Configuration { * @memberof Configuration */ retryParams?: RetryParams; + /** + * telemetry configuration + * + * @type {TelemetryConfiguration} + * @memberof Configuration + */ + telemetryConfig: TelemetryConfiguration; constructor(params: UserConfigurationParams = {} as unknown as UserConfigurationParams) { this.apiScheme = params.apiScheme || this.apiScheme; @@ -168,6 +177,7 @@ export class Configuration { this.baseOptions = baseOptions; this.retryParams = params.retryParams; + this.telemetryConfig = params.telemetryConfig || new TelemetryConfiguration(); } /** diff --git a/credentials/credentials.ts b/credentials/credentials.ts index 096e0f7..930992f 100644 --- a/credentials/credentials.ts +++ b/credentials/credentials.ts @@ -16,14 +16,14 @@ import globalAxios, { AxiosInstance } from "axios"; import { assertParamExists, isWellFormedUriString } from "../validation"; import { FgaApiAuthenticationError, FgaApiError, FgaError, FgaValidationError } from "../errors"; import { attemptHttpRequest } from "../common"; -import { buildAttributes } from "../telemetry"; import { ApiTokenConfig, AuthCredentialsConfig, ClientCredentialsConfig, CredentialsMethod } from "./types"; -import { Counter, metrics } from "@opentelemetry/api"; +import { TelemetryAttributes } from "../telemetry/attributes"; +import { TelemetryMetrics } from "../telemetry/metrics"; +import { TelemetryCounters } from "../telemetry/counters"; export class Credentials { private accessToken?: string; private accessTokenExpiryDate?: Date; - private tokenCounter?: Counter; public static init(configuration: { credentials: AuthCredentialsConfig }): Credentials { return new Credentials(configuration.credentials); @@ -51,11 +51,6 @@ export class Credentials { } } break; - case CredentialsMethod.ClientCredentials: { - const meter = metrics.getMeter("@openfga/sdk", "0.6.3"); - this.tokenCounter = meter.createCounter("fga-client.credentials.request"); - break; - } case CredentialsMethod.None: default: break; @@ -165,7 +160,25 @@ export class Credentials { this.accessTokenExpiryDate = new Date(Date.now() + response.data.expires_in * 1000); } - this.tokenCounter?.add(1, buildAttributes(response, this.authConfig)); + const telemetryAttributes = new TelemetryAttributes(); + const telemetryMetrics = new TelemetryMetrics(); + + let attributes = {}; + + attributes = telemetryAttributes.fromRequest({ + credentials: clientCredentials, + // resendCount: 0, // TODO: implement resend count tracking, not available in the current context + attributes, + }); + + attributes = telemetryAttributes.fromResponse({ + response, + credentials: clientCredentials, + attributes, + }); + + attributes = telemetryAttributes.prepare(attributes); + telemetryMetrics.counter(TelemetryCounters.credentialsRequest, 1, attributes); return this.accessToken; } catch (err: unknown) { diff --git a/index.ts b/index.ts index c17e345..291e294 100644 --- a/index.ts +++ b/index.ts @@ -18,6 +18,11 @@ export * from "./client"; export * from "./apiModel"; export { Configuration, UserConfigurationParams, GetDefaultRetryParams } from "./configuration"; export { Credentials, CredentialsMethod } from "./credentials"; +export * from "./telemetry/attributes"; +export * from "./telemetry/configuration"; +export * from "./telemetry/counters"; +export * from "./telemetry/histograms"; +export * from "./telemetry/metrics"; export * from "./errors"; diff --git a/package-lock.json b/package-lock.json index a717980..b8ce6b7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,6 @@ "license": "Apache-2.0", "dependencies": { "@opentelemetry/api": "^1.9.0", - "@opentelemetry/semantic-conventions": "^1.26.0", "axios": "^1.7.5", "tiny-async-pool": "^2.1.0" }, @@ -1318,14 +1317,6 @@ "node": ">=8.0.0" } }, - "node_modules/@opentelemetry/semantic-conventions": { - "version": "1.27.0", - "resolved": "https://registry.npmjs.org/@opentelemetry/semantic-conventions/-/semantic-conventions-1.27.0.tgz", - "integrity": "sha512-sAay1RrB+ONOem0OZanAR1ZI/k7yDpnOQSQmTMuGImUQb2y8EbSaCJ94FQluM74xoU03vlb2d2U90hZluL6nQg==", - "engines": { - "node": ">=14" - } - }, "node_modules/@sinclair/typebox": { "version": "0.27.8", "resolved": "https://registry.npmjs.org/@sinclair/typebox/-/typebox-0.27.8.tgz", @@ -6288,11 +6279,6 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.9.0.tgz", "integrity": "sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg==" }, - "@opentelemetry/semantic-conventions": { - "version": "1.27.0", - "resolved": "https://registry.npmjs.org/@opentelemetry/semantic-conventions/-/semantic-conventions-1.27.0.tgz", - "integrity": "sha512-sAay1RrB+ONOem0OZanAR1ZI/k7yDpnOQSQmTMuGImUQb2y8EbSaCJ94FQluM74xoU03vlb2d2U90hZluL6nQg==" - }, "@sinclair/typebox": { "version": "0.27.8", "resolved": "https://registry.npmjs.org/@sinclair/typebox/-/typebox-0.27.8.tgz", diff --git a/package.json b/package.json index c78f898..c6fa2ba 100644 --- a/package.json +++ b/package.json @@ -23,7 +23,6 @@ }, "dependencies": { "@opentelemetry/api": "^1.9.0", - "@opentelemetry/semantic-conventions": "^1.26.0", "axios": "^1.7.5", "tiny-async-pool": "^2.1.0" }, diff --git a/telemetry.ts b/telemetry.ts deleted file mode 100644 index 3c34e08..0000000 --- a/telemetry.ts +++ /dev/null @@ -1,71 +0,0 @@ -/** - * JavaScript and Node.js SDK for OpenFGA - * - * API version: 1.x - * Website: https://openfga.dev - * Documentation: https://openfga.dev/docs - * Support: https://openfga.dev/community - * License: [Apache-2.0](https://github.com/openfga/js-sdk/blob/main/LICENSE) - * - * NOTE: This file was auto generated by OpenAPI Generator (https://openapi-generator.tech). DO NOT EDIT. - */ - - -import { AxiosResponse } from "axios"; -import { Attributes } from "@opentelemetry/api"; -import { SEMATTRS_HTTP_HOST, SEMATTRS_HTTP_METHOD, SEMATTRS_HTTP_STATUS_CODE } from "@opentelemetry/semantic-conventions"; -import { AuthCredentialsConfig, CredentialsMethod } from "./credentials/types"; - -/** - * Builds an object of attributes that can be used to report alongside an OpenTelemetry metric event. - * - * @param response - The Axios response object, used to add data like HTTP status, host, method, and headers. - * @param credentials - The credentials object, used to add data like the ClientID when using ClientCredentials. - * @param methodAttributes - Extra attributes that the method (i.e. check, listObjects) wishes to have included. Any custom attributes should use the common names. - * @returns {Attributes} - */ - -export const buildAttributes = function buildAttributes(response: AxiosResponse | undefined, credentials: AuthCredentialsConfig, methodAttributes: Record = {}): Attributes { - const attributes: Attributes = { - ...methodAttributes, - }; - - if (response?.status) { - attributes[SEMATTRS_HTTP_STATUS_CODE] = response.status; - } - - if (response?.request) { - attributes[SEMATTRS_HTTP_METHOD] = response.request.method; - attributes[SEMATTRS_HTTP_HOST] = response.request.host; - } - - if (response?.headers) { - const modelId = response.headers["openfga-authorization-model-id"]; - if (modelId !== undefined) { - attributes[attributeNames.responseModelId] = modelId; - } - } - - if (credentials?.method === CredentialsMethod.ClientCredentials) { - attributes[attributeNames.requestClientId] = credentials.config.clientId; - } - - return attributes; -}; -/** - * Common attribute names - */ - -export const attributeNames = { - // Attributes associated with the request made - requestModelId: "fga-client.request.model_id", - requestMethod: "fga-client.request.method", - requestStoreId: "fga-client.request.store_id", - requestClientId: "fga-client.request.client_id", - - // Attributes associated with the response - responseModelId: "fga-client.response.model_id", - - // Attributes associated with specific actions - user: "fga-client.user" -}; diff --git a/telemetry/attributes.ts b/telemetry/attributes.ts new file mode 100644 index 0000000..9f6c1de --- /dev/null +++ b/telemetry/attributes.ts @@ -0,0 +1,103 @@ +import { URL } from 'url'; + +export interface TelemetryAttribute { + name: string; +} + +export class TelemetryAttributes { + static fgaClientRequestClientId: TelemetryAttribute = { name: 'fga-client.request.client_id' }; + static fgaClientRequestMethod: TelemetryAttribute = { name: 'fga-client.request.method' }; + static fgaClientRequestModelId: TelemetryAttribute = { name: 'fga-client.request.model_id' }; + static fgaClientRequestStoreId: TelemetryAttribute = { name: 'fga-client.request.store_id' }; + static fgaClientResponseModelId: TelemetryAttribute = { name: 'fga-client.response.model_id' }; + static fgaClientUser: TelemetryAttribute = { name: 'fga-client.user' }; + static httpClientRequestDuration: TelemetryAttribute = { name: 'http.client.request.duration' }; + static httpHost: TelemetryAttribute = { name: 'http.host' }; + static httpRequestMethod: TelemetryAttribute = { name: 'http.request.method' }; + static httpRequestResendCount: TelemetryAttribute = { name: 'http.request.resend_count' }; + static httpResponseStatusCode: TelemetryAttribute = { name: 'http.response.status_code' }; + static httpServerRequestDuration: TelemetryAttribute = { name: 'http.server.request.duration' }; + static urlScheme: TelemetryAttribute = { name: 'url.scheme' }; + static urlFull: TelemetryAttribute = { name: 'url.full' }; + static userAgentOriginal: TelemetryAttribute = { name: 'user_agent.original' }; + + prepare( + attributes?: Record, + filter?: string[] + ): Record { + const response: Record = {}; + + if (attributes) { + Object.entries(attributes).forEach(([key, value]) => { + if (filter && !filter.includes(key)) return; + response[key] = value; + }); + } + + return Object.fromEntries(Object.entries(response).sort()); + } + + fromRequest({ + userAgent, + fgaMethod, + httpMethod, + url, + resendCount, + start, + credentials, + attributes = {}, + }: { + userAgent?: string; + fgaMethod?: string; + httpMethod?: string; + url?: string; + resendCount?: number; + start?: number; + credentials?: any; + attributes?: Record; + }): Record { + if (fgaMethod) attributes[TelemetryAttributes.fgaClientRequestMethod.name] = fgaMethod; + if (userAgent) attributes[TelemetryAttributes.userAgentOriginal.name] = userAgent; + if (httpMethod) attributes[TelemetryAttributes.httpRequestMethod.name] = httpMethod; + + if (url) { + const parsedUrl = new URL(url); + attributes[TelemetryAttributes.httpHost.name] = parsedUrl.hostname; + attributes[TelemetryAttributes.urlScheme.name] = parsedUrl.protocol; + attributes[TelemetryAttributes.urlFull.name] = url; + } + + if (start) attributes[TelemetryAttributes.httpClientRequestDuration.name] = Date.now() - start; + if (resendCount) attributes[TelemetryAttributes.httpRequestResendCount.name] = resendCount; + if (credentials && credentials.method === 'client_credentials') { + attributes[TelemetryAttributes.fgaClientRequestClientId.name] = credentials.configuration.clientId; + } + + return attributes; + } + + fromResponse({ + response, + credentials, + attributes = {}, + }: { + response: any; + credentials?: any; + attributes?: Record; + }): Record { + if (response?.status) attributes[TelemetryAttributes.httpResponseStatusCode.name] = response.status; + + const responseHeaders = response?.headers || {}; + const responseModelId = responseHeaders['openfga-authorization-model-id']; + const responseQueryDuration = responseHeaders['fga-query-duration-ms']; + + if (responseModelId) attributes[TelemetryAttributes.fgaClientResponseModelId.name] = responseModelId; + if (responseQueryDuration) attributes[TelemetryAttributes.httpServerRequestDuration.name] = responseQueryDuration; + + if (credentials && credentials.method === 'client_credentials') { + attributes[TelemetryAttributes.fgaClientRequestClientId.name] = credentials.configuration.clientId; + } + + return attributes; + } +} diff --git a/telemetry/configuration.ts b/telemetry/configuration.ts new file mode 100644 index 0000000..17a7b3e --- /dev/null +++ b/telemetry/configuration.ts @@ -0,0 +1,53 @@ +import { TelemetryAttributes, TelemetryAttribute } from './attributes'; + +export class TelemetryMetricConfiguration { + constructor( + public enabled: boolean = true, + public attrFgaClientRequestClientId: boolean = true, + public attrFgaClientRequestMethod: boolean = true, + public attrFgaClientRequestModelId: boolean = true, + public attrFgaClientRequestStoreId: boolean = true, + public attrFgaClientResponseModelId: boolean = true, + public attrFgaClientUser: boolean = false, + public attrHttpClientRequestDuration: boolean = false, + public attrHttpHost: boolean = true, + public attrHttpRequestMethod: boolean = true, + public attrHttpRequestResendCount: boolean = true, + public attrHttpResponseStatusCode: boolean = true, + public attrHttpServerRequestDuration: boolean = false, + public attrUrlScheme: boolean = true, + public attrUrlFull: boolean = true, + public attrUserAgentOriginal: boolean = true + ) {} + + attributes(): Record { + const enabled: Record = {}; + + Object.entries(this).forEach(([key, value]) => { + if (key.startsWith('attr') && value) { + let attrKey = key.replace('attr', '') as keyof typeof TelemetryAttributes; + attrKey = attrKey.charAt(0).toLowerCase() + attrKey.slice(1); + + const telemetryAttribute = TelemetryAttributes[attrKey as keyof typeof TelemetryAttributes] as TelemetryAttribute; + + if (telemetryAttribute) { + enabled[telemetryAttribute.name] = true; + } + } + }); + + return enabled; + } +} + +export class TelemetryMetricsConfiguration { + constructor( + public counterCredentialsRequest: TelemetryMetricConfiguration = new TelemetryMetricConfiguration(), + public histogramRequestDuration: TelemetryMetricConfiguration = new TelemetryMetricConfiguration(), + public histogramQueryDuration: TelemetryMetricConfiguration = new TelemetryMetricConfiguration() + ) {} +} + +export class TelemetryConfiguration { + constructor(public metrics: TelemetryMetricsConfiguration = new TelemetryMetricsConfiguration()) {} +} diff --git a/telemetry/counters.ts b/telemetry/counters.ts new file mode 100644 index 0000000..3ed6cd8 --- /dev/null +++ b/telemetry/counters.ts @@ -0,0 +1,13 @@ +export interface TelemetryCounter { + name: string; + unit: string; + description: string; +} + +export class TelemetryCounters { + static credentialsRequest: TelemetryCounter = { + name: 'fga-client.credentials.request', + unit: 'milliseconds', + description: 'The number of times an access token is requested.', + }; +} diff --git a/telemetry/histograms.ts b/telemetry/histograms.ts new file mode 100644 index 0000000..50d4558 --- /dev/null +++ b/telemetry/histograms.ts @@ -0,0 +1,19 @@ +export interface TelemetryHistogram { + name: string; + unit: string; + description: string; +} + +export class TelemetryHistograms { + static requestDuration: TelemetryHistogram = { + name: 'fga-client.request.duration', + unit: 'milliseconds', + description: 'How long it took for a request to be fulfilled.', + }; + + static queryDuration: TelemetryHistogram = { + name: 'fga-client.query.duration', + unit: 'milliseconds', + description: 'How long it took to perform a query request.', + }; +} diff --git a/telemetry/metrics.ts b/telemetry/metrics.ts new file mode 100644 index 0000000..2334eb5 --- /dev/null +++ b/telemetry/metrics.ts @@ -0,0 +1,59 @@ +import { Counter, Histogram, Meter, ValueType } from '@opentelemetry/api'; +import { TelemetryConfiguration, TelemetryMetricsConfiguration } from './configuration'; +import { TelemetryCounters, TelemetryCounter } from './counters'; +import { TelemetryHistograms, TelemetryHistogram } from './histograms'; +import { TelemetryAttributes, TelemetryAttribute } from './attributes'; +import { metrics } from "@opentelemetry/api"; + +export class TelemetryMetrics { + private _meter: Meter | null = null; + private _counters: Record = {}; + private _histograms: Record = {}; + + constructor( + meter?: Meter, + counters?: Record, + histograms?: Record + ) { + this._meter = meter || null; + this._counters = counters || {}; + this._histograms = histograms || {}; + } + + meter(): Meter { + if (!this._meter) { + this._meter = metrics.getMeter("@openfga/sdk", "0.6.3") + } + return this._meter; + } + + counter(counter: TelemetryCounter, value?: number, attributes?: Record): Counter { + if (!this._counters[counter.name]) { + this._counters[counter.name] = this.meter().createCounter(counter.name, { + description: counter.description, + unit: counter.unit, + }); + } + + if (value !== undefined) { + this._counters[counter.name].add(value, attributes); + } + + return this._counters[counter.name]; + } + + histogram(histogram: TelemetryHistogram, value?: number, attributes?: Record): Histogram { + if (!this._histograms[histogram.name]) { + this._histograms[histogram.name] = this.meter().createHistogram(histogram.name, { + description: histogram.description, + unit: histogram.unit, + }); + } + + if (value !== undefined) { + this._histograms[histogram.name].record(value, attributes); + } + + return this._histograms[histogram.name]; + } +} diff --git a/tests/telemetry/attributes.test.ts b/tests/telemetry/attributes.test.ts new file mode 100644 index 0000000..02dcc43 --- /dev/null +++ b/tests/telemetry/attributes.test.ts @@ -0,0 +1,69 @@ +import { TelemetryAttributes } from '../../telemetry/attributes'; + +describe('TelemetryAttributes', () => { + let telemetryAttributes: TelemetryAttributes; + + beforeEach(() => { + telemetryAttributes = new TelemetryAttributes(); + }); + + test('should prepare attributes correctly', () => { + const attributes = { + 'fga-client.request.client_id': 'test-client-id', + 'http.host': 'example.com', + }; + + const filter = ['fga-client.request.client_id']; + const prepared = telemetryAttributes.prepare(attributes, filter); + + expect(prepared).toEqual({ 'fga-client.request.client_id': 'test-client-id' }); + }); + + test('should create attributes from request correctly', () => { + const result = telemetryAttributes.fromRequest({ + userAgent: 'Mozilla/5.0', + fgaMethod: 'GET', + httpMethod: 'POST', + url: 'https://example.com', + resendCount: 2, + start: 1000, + credentials: { method: 'client_credentials', configuration: { clientId: 'client-id' } }, + }); + + expect(result['user_agent.original']).toEqual('Mozilla/5.0'); + expect(result['fga-client.request.method']).toEqual('GET'); + expect(result['http.request.method']).toEqual('POST'); + expect(result['http.host']).toEqual('example.com'); + expect(result['url.scheme']).toEqual('https:'); + }); + + test('should create attributes from response correctly', () => { + const response = { status: 200, headers: { 'openfga-authorization-model-id': 'model-id', 'fga-query-duration-ms': '10' } }; + const result = telemetryAttributes.fromResponse({ response }); + + // Verify line 90 is covered - status is correctly set + expect(result['http.response.status_code']).toEqual(200); + expect(result['fga-client.response.model_id']).toEqual('model-id'); + expect(result['http.server.request.duration']).toEqual('10'); + }); + + test('should handle response without status correctly', () => { + const response = { headers: { 'openfga-authorization-model-id': 'model-id', 'fga-query-duration-ms': '10' } }; + const result = telemetryAttributes.fromResponse({ response }); + + // Verify that no status code is set when response does not have a status + expect(result['http.response.status_code']).toBeUndefined(); + expect(result['fga-client.response.model_id']).toEqual('model-id'); + expect(result['http.server.request.duration']).toEqual('10'); + }); + + test('should create attributes from response with client credentials', () => { + const response = { status: 200, headers: {} }; + const credentials = { method: 'client_credentials', configuration: { clientId: 'client-id' } }; + const result = telemetryAttributes.fromResponse({ response, credentials }); + + // Check that the client ID is set correctly from the credentials + expect(result['http.response.status_code']).toEqual(200); + expect(result['fga-client.request.client_id']).toEqual('client-id'); + }); +}); diff --git a/tests/telemetry/configuration.test.ts b/tests/telemetry/configuration.test.ts new file mode 100644 index 0000000..d368490 --- /dev/null +++ b/tests/telemetry/configuration.test.ts @@ -0,0 +1,68 @@ +import { TelemetryMetricConfiguration, TelemetryConfiguration, TelemetryMetricsConfiguration } from '../../telemetry/configuration'; +import { TelemetryAttributes } from '../../telemetry/attributes'; + +describe('TelemetryMetricConfiguration', () => { + test('should create a default TelemetryMetricConfiguration instance', () => { + const config = new TelemetryMetricConfiguration(); + + expect(config.enabled).toBe(true); + expect(config.attrFgaClientRequestClientId).toBe(true); + expect(config.attrFgaClientRequestMethod).toBe(true); + expect(config.attrFgaClientRequestModelId).toBe(true); + expect(config.attrFgaClientRequestStoreId).toBe(true); + expect(config.attrFgaClientResponseModelId).toBe(true); + expect(config.attrFgaClientUser).toBe(false); + }); + + test('should return correct attributes based on enabled properties', () => { + const config = new TelemetryMetricConfiguration( + true, // enabled + true, // attrFgaClientRequestClientId + false, // attrFgaClientRequestMethod + true, // attrFgaClientRequestModelId + false, // attrFgaClientRequestStoreId + true, // attrFgaClientResponseModelId + false, // attrFgaClientUser + true, // attrHttpClientRequestDuration + false, // attrHttpHost + true, // attrHttpRequestMethod + true, // attrHttpRequestResendCount + false, // attrHttpResponseStatusCode + true, // attrHttpServerRequestDuration + false, // attrUrlScheme + true, // attrUrlFull + false, // attrUserAgentOriginal + ); + + const attributes = config.attributes(); + + expect(attributes).toEqual({ + [TelemetryAttributes.fgaClientRequestClientId.name]: true, + [TelemetryAttributes.fgaClientRequestModelId.name]: true, + [TelemetryAttributes.fgaClientResponseModelId.name]: true, + [TelemetryAttributes.httpClientRequestDuration.name]: true, + [TelemetryAttributes.httpRequestMethod.name]: true, + [TelemetryAttributes.httpRequestResendCount.name]: true, + [TelemetryAttributes.httpServerRequestDuration.name]: true, + [TelemetryAttributes.urlFull.name]: true, + }); + }); +}); + +describe('TelemetryConfiguration', () => { + test('should create a default TelemetryConfiguration instance', () => { + const config = new TelemetryConfiguration(); + + expect(config.metrics).toBeInstanceOf(TelemetryMetricsConfiguration); + expect(config.metrics.counterCredentialsRequest.enabled).toBe(true); + }); + + test('should allow overriding telemetry configuration options', () => { + const customMetrics = new TelemetryMetricsConfiguration(); + customMetrics.counterCredentialsRequest.enabled = false; + + const config = new TelemetryConfiguration(customMetrics); + + expect(config.metrics.counterCredentialsRequest.enabled).toBe(false); + }); +}); diff --git a/tests/telemetry/counters.test.ts b/tests/telemetry/counters.test.ts new file mode 100644 index 0000000..45fad6f --- /dev/null +++ b/tests/telemetry/counters.test.ts @@ -0,0 +1,9 @@ +import { TelemetryCounters } from '../../telemetry/counters'; + +describe('TelemetryCounters', () => { + test('should have correct counter details', () => { + expect(TelemetryCounters.credentialsRequest.name).toBe('fga-client.credentials.request'); + expect(TelemetryCounters.credentialsRequest.unit).toBe('milliseconds'); + expect(TelemetryCounters.credentialsRequest.description).toBe('The number of times an access token is requested.'); + }); +}); diff --git a/tests/telemetry/histograms.test.ts b/tests/telemetry/histograms.test.ts new file mode 100644 index 0000000..f31d915 --- /dev/null +++ b/tests/telemetry/histograms.test.ts @@ -0,0 +1,15 @@ +import { TelemetryHistograms } from '../../telemetry/histograms'; + +describe('TelemetryHistograms', () => { + test('should have correct histogram details for request duration', () => { + expect(TelemetryHistograms.requestDuration.name).toBe('fga-client.request.duration'); + expect(TelemetryHistograms.requestDuration.unit).toBe('milliseconds'); + expect(TelemetryHistograms.requestDuration.description).toBe('How long it took for a request to be fulfilled.'); + }); + + test('should have correct histogram details for query duration', () => { + expect(TelemetryHistograms.queryDuration.name).toBe('fga-client.query.duration'); + expect(TelemetryHistograms.queryDuration.unit).toBe('milliseconds'); + expect(TelemetryHistograms.queryDuration.description).toBe('How long it took to perform a query request.'); + }); +}); diff --git a/tests/telemetry/metrics.test.ts b/tests/telemetry/metrics.test.ts new file mode 100644 index 0000000..d49f37a --- /dev/null +++ b/tests/telemetry/metrics.test.ts @@ -0,0 +1,42 @@ +import { TelemetryMetrics } from '../../telemetry/metrics'; +import { TelemetryCounters } from '../../telemetry/counters'; +import { TelemetryHistograms } from '../../telemetry/histograms'; +import { TelemetryAttributes } from '../../telemetry/attributes'; + +jest.mock('@opentelemetry/api', () => ({ + metrics: { + getMeter: jest.fn().mockReturnValue({ + createCounter: jest.fn().mockReturnValue({ add: jest.fn() }), + createHistogram: jest.fn().mockReturnValue({ record: jest.fn() }), + }), + }, +})); + +describe('TelemetryMetrics', () => { + let telemetryMetrics: TelemetryMetrics; + + beforeEach(() => { + telemetryMetrics = new TelemetryMetrics(); + }); + + test('should create a counter and add a value', () => { + const counter = telemetryMetrics.counter(TelemetryCounters.credentialsRequest, 5); + + expect(counter).toBeDefined(); + expect(counter.add).toHaveBeenCalledWith(5, undefined); + }); + + test('should create a histogram and record a value', () => { + const histogram = telemetryMetrics.histogram(TelemetryHistograms.requestDuration, 200); + + expect(histogram).toBeDefined(); + expect(histogram.record).toHaveBeenCalledWith(200, undefined); + }); + + test('should handle creating metrics with custom attributes', () => { + const attributes = new TelemetryAttributes().prepare({ 'http.host': 'example.com' }); + const counter = telemetryMetrics.counter(TelemetryCounters.credentialsRequest, 3, attributes); + + expect(counter.add).toHaveBeenCalledWith(3, attributes); + }); +}); From d458b937406ba8562908c17821d0af135c5b6430 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Fri, 13 Sep 2024 12:59:59 -0500 Subject: [PATCH 02/30] use enums for telemetry attributes --- api.ts | 80 +++++++++++++------------- common.ts | 12 ++-- telemetry/attributes.ts | 83 ++++++++++++++------------- telemetry/configuration.ts | 55 +++++++----------- tests/telemetry/attributes.test.ts | 30 +++++++++- tests/telemetry/configuration.test.ts | 62 ++++++++++---------- 6 files changed, 165 insertions(+), 157 deletions(-) diff --git a/api.ts b/api.ts index 189841a..984d3ce 100644 --- a/api.ts +++ b/api.ts @@ -109,7 +109,7 @@ import { WriteRequestDeletes, WriteRequestWrites, } from "./apiModel"; -import { TelemetryAttributes } from "./telemetry/attributes"; +import { TelemetryAttribute, TelemetryAttributes } from "./telemetry/attributes"; /** @@ -759,10 +759,10 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async check(storeId: string, body: CheckRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.check(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "Check", - [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId ?? "", - [TelemetryAttributes.fgaClientRequestModelId.name]: body.authorization_model_id ?? "", - [TelemetryAttributes.fgaClientUser.name]: body.tuple_key.user + [TelemetryAttribute.FgaClientRequestMethod]: "Check", + [TelemetryAttribute.FgaClientRequestStoreId]: storeId ?? "", + [TelemetryAttribute.FgaClientRequestModelId]: body.authorization_model_id ?? "", + [TelemetryAttribute.FgaClientUser]: body.tuple_key.user }); }, /** @@ -775,7 +775,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async createStore(body: CreateStoreRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.createStore(body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "createStore", + [TelemetryAttribute.FgaClientRequestMethod]: "createStore", }); }, /** @@ -788,8 +788,8 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async deleteStore(storeId: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.deleteStore(storeId, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "deleteStore", - [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, + [TelemetryAttribute.FgaClientRequestMethod]: "deleteStore", + [TelemetryAttribute.FgaClientRequestStoreId]: storeId, }); }, /** @@ -803,9 +803,9 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async expand(storeId: string, body: ExpandRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.expand(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "expand", - [TelemetryAttributes.fgaClientRequestModelId.name]: body.authorization_model_id ?? "", - [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId ?? "", + [TelemetryAttribute.FgaClientRequestMethod]: "expand", + [TelemetryAttribute.FgaClientRequestModelId]: body.authorization_model_id ?? "", + [TelemetryAttribute.FgaClientRequestStoreId]: storeId ?? "", }); }, /** @@ -818,8 +818,8 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async getStore(storeId: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.getStore(storeId, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "getStore", - [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, + [TelemetryAttribute.FgaClientRequestMethod]: "getStore", + [TelemetryAttribute.FgaClientRequestStoreId]: storeId, }); }, /** @@ -833,10 +833,10 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async listObjects(storeId: string, body: ListObjectsRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.listObjects(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "listObjects", - [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId ?? "", - [TelemetryAttributes.fgaClientRequestModelId.name]: body.authorization_model_id ?? "", - [TelemetryAttributes.fgaClientUser.name]: body.user + [TelemetryAttribute.FgaClientRequestMethod]: "listObjects", + [TelemetryAttribute.FgaClientRequestStoreId]: storeId ?? "", + [TelemetryAttribute.FgaClientRequestModelId]: body.authorization_model_id ?? "", + [TelemetryAttribute.FgaClientUser]: body.user }); }, /** @@ -850,7 +850,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async listStores(pageSize?: number, continuationToken?: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.listStores(pageSize, continuationToken, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "listStores", + [TelemetryAttribute.FgaClientRequestMethod]: "listStores", }); }, /** @@ -864,9 +864,9 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async listUsers(storeId: string, body: ListUsersRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.listUsers(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "listUsers", - [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId ?? "", - [TelemetryAttributes.fgaClientRequestModelId.name]: body.authorization_model_id ?? "", + [TelemetryAttribute.FgaClientRequestMethod]: "listUsers", + [TelemetryAttribute.FgaClientRequestStoreId]: storeId ?? "", + [TelemetryAttribute.FgaClientRequestModelId]: body.authorization_model_id ?? "", }); }, /** @@ -880,8 +880,8 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async read(storeId: string, body: ReadRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.read(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "read", - [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, + [TelemetryAttribute.FgaClientRequestMethod]: "read", + [TelemetryAttribute.FgaClientRequestStoreId]: storeId, }); }, /** @@ -895,9 +895,9 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async readAssertions(storeId: string, authorizationModelId: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.readAssertions(storeId, authorizationModelId, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "readAssertions", - [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, - [TelemetryAttributes.fgaClientRequestModelId.name]: authorizationModelId, + [TelemetryAttribute.FgaClientRequestMethod]: "readAssertions", + [TelemetryAttribute.FgaClientRequestStoreId]: storeId, + [TelemetryAttribute.FgaClientRequestModelId]: authorizationModelId, }); }, /** @@ -911,8 +911,8 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async readAuthorizationModel(storeId: string, id: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.readAuthorizationModel(storeId, id, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "readAuthorizationModel", - [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, + [TelemetryAttribute.FgaClientRequestMethod]: "readAuthorizationModel", + [TelemetryAttribute.FgaClientRequestStoreId]: storeId, }); }, /** @@ -927,8 +927,8 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async readAuthorizationModels(storeId: string, pageSize?: number, continuationToken?: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.readAuthorizationModels(storeId, pageSize, continuationToken, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "readAuthorizationModels", - [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, + [TelemetryAttribute.FgaClientRequestMethod]: "readAuthorizationModels", + [TelemetryAttribute.FgaClientRequestStoreId]: storeId, }); }, /** @@ -944,8 +944,8 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async readChanges(storeId: string, type?: string, pageSize?: number, continuationToken?: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.readChanges(storeId, type, pageSize, continuationToken, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "readChanges", - [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, + [TelemetryAttribute.FgaClientRequestMethod]: "readChanges", + [TelemetryAttribute.FgaClientRequestStoreId]: storeId, }); }, /** @@ -959,9 +959,9 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async write(storeId: string, body: WriteRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.write(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "write", - [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId ?? "", - [TelemetryAttributes.fgaClientRequestModelId.name]: body.authorization_model_id ?? "", + [TelemetryAttribute.FgaClientRequestMethod]: "write", + [TelemetryAttribute.FgaClientRequestStoreId]: storeId ?? "", + [TelemetryAttribute.FgaClientRequestModelId]: body.authorization_model_id ?? "", }); }, /** @@ -976,9 +976,9 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async writeAssertions(storeId: string, authorizationModelId: string, body: WriteAssertionsRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.writeAssertions(storeId, authorizationModelId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "writeAssertions", - [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, - [TelemetryAttributes.fgaClientRequestModelId.name]: authorizationModelId, + [TelemetryAttribute.FgaClientRequestMethod]: "writeAssertions", + [TelemetryAttribute.FgaClientRequestStoreId]: storeId, + [TelemetryAttribute.FgaClientRequestModelId]: authorizationModelId, }); }, /** @@ -992,8 +992,8 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async writeAuthorizationModel(storeId: string, body: WriteAuthorizationModelRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.writeAuthorizationModel(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttributes.fgaClientRequestMethod.name]: "writeAuthorizationModel", - [TelemetryAttributes.fgaClientRequestStoreId.name]: storeId, + [TelemetryAttribute.FgaClientRequestMethod]: "writeAuthorizationModel", + [TelemetryAttribute.FgaClientRequestStoreId]: storeId, }); }, }; diff --git a/common.ts b/common.ts index 6b71eca..fdf2566 100644 --- a/common.ts +++ b/common.ts @@ -25,7 +25,7 @@ import { FgaError } from "./errors"; import { setNotEnumerableProperty } from "./utils"; -import { TelemetryAttributes } from "./telemetry/attributes"; +import { TelemetryAttribute, TelemetryAttributes } from "./telemetry/attributes"; import { TelemetryMetrics } from "./telemetry/metrics"; import { TelemetryHistograms } from "./telemetry/histograms"; @@ -226,24 +226,24 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst attributes, }); - if (attributes[TelemetryAttributes.httpServerRequestDuration.name]) { + if (attributes[TelemetryAttribute.HttpServerRequestDuration]) { telemetryMetrics.histogram( TelemetryHistograms.queryDuration, - parseInt(attributes[TelemetryAttributes.httpServerRequestDuration.name] as string, 10), + parseInt(attributes[TelemetryAttribute.HttpServerRequestDuration] as string, 10), telemetryAttributes.prepare( attributes, - Object.keys(configuration.telemetryConfig.metrics.histogramQueryDuration.attributes()) + configuration.telemetryConfig.metrics.histogramQueryDuration.attributes ) ); } - if (attributes[TelemetryAttributes.httpClientRequestDuration.name]) { + if (attributes[TelemetryAttribute.HttpClientRequestDuration]) { telemetryMetrics.histogram( TelemetryHistograms.requestDuration, Date.now() - start, telemetryAttributes.prepare( attributes, - Object.keys(configuration.telemetryConfig.metrics.histogramRequestDuration.attributes()) + configuration.telemetryConfig.metrics.histogramRequestDuration.attributes ) ); } diff --git a/telemetry/attributes.ts b/telemetry/attributes.ts index 9f6c1de..2b71518 100644 --- a/telemetry/attributes.ts +++ b/telemetry/attributes.ts @@ -1,40 +1,41 @@ import { URL } from 'url'; -export interface TelemetryAttribute { - name: string; +export enum TelemetryAttribute { + FgaClientRequestClientId = "fga-client.request.client_id", + FgaClientRequestMethod = "fga-client.request.method", + FgaClientRequestModelId = "fga-client.request.model_id", + FgaClientRequestStoreId = "fga-client.request.store_id", + FgaClientResponseModelId = "fga-client.response.model_id", + FgaClientUser = "fga-client.user", + // remove this attribute, keep as metric + HttpClientRequestDuration = "http.client.request.duration", + HttpHost = "http.host", + HttpRequestMethod = "http.request.method", + HttpRequestResendCount = "http.request.resend_count", + HttpResponseStatusCode = "http.response.status_code", + // remove this attribute, keep as metric + HttpServerRequestDuration = "http.server.request.duration", + UrlScheme = "url.scheme", + UrlFull = "url.full", + UserAgentOriginal = "user_agent.original", } export class TelemetryAttributes { - static fgaClientRequestClientId: TelemetryAttribute = { name: 'fga-client.request.client_id' }; - static fgaClientRequestMethod: TelemetryAttribute = { name: 'fga-client.request.method' }; - static fgaClientRequestModelId: TelemetryAttribute = { name: 'fga-client.request.model_id' }; - static fgaClientRequestStoreId: TelemetryAttribute = { name: 'fga-client.request.store_id' }; - static fgaClientResponseModelId: TelemetryAttribute = { name: 'fga-client.response.model_id' }; - static fgaClientUser: TelemetryAttribute = { name: 'fga-client.user' }; - static httpClientRequestDuration: TelemetryAttribute = { name: 'http.client.request.duration' }; - static httpHost: TelemetryAttribute = { name: 'http.host' }; - static httpRequestMethod: TelemetryAttribute = { name: 'http.request.method' }; - static httpRequestResendCount: TelemetryAttribute = { name: 'http.request.resend_count' }; - static httpResponseStatusCode: TelemetryAttribute = { name: 'http.response.status_code' }; - static httpServerRequestDuration: TelemetryAttribute = { name: 'http.server.request.duration' }; - static urlScheme: TelemetryAttribute = { name: 'url.scheme' }; - static urlFull: TelemetryAttribute = { name: 'url.full' }; - static userAgentOriginal: TelemetryAttribute = { name: 'user_agent.original' }; - prepare( attributes?: Record, - filter?: string[] + filter?: Set ): Record { - const response: Record = {}; - - if (attributes) { - Object.entries(attributes).forEach(([key, value]) => { - if (filter && !filter.includes(key)) return; - response[key] = value; - }); + attributes = attributes || {}; + filter = filter || new Set(); + const result: Record = {}; + + for (const key in attributes) { + if (filter.has(key as TelemetryAttribute)) { + result[key] = attributes[key]; + } } - - return Object.fromEntries(Object.entries(response).sort()); + + return result; } fromRequest({ @@ -56,21 +57,21 @@ export class TelemetryAttributes { credentials?: any; attributes?: Record; }): Record { - if (fgaMethod) attributes[TelemetryAttributes.fgaClientRequestMethod.name] = fgaMethod; - if (userAgent) attributes[TelemetryAttributes.userAgentOriginal.name] = userAgent; - if (httpMethod) attributes[TelemetryAttributes.httpRequestMethod.name] = httpMethod; + if (fgaMethod) attributes[TelemetryAttribute.FgaClientRequestMethod] = fgaMethod; + if (userAgent) attributes[TelemetryAttribute.UserAgentOriginal] = userAgent; + if (httpMethod) attributes[TelemetryAttribute.HttpRequestMethod] = httpMethod; if (url) { const parsedUrl = new URL(url); - attributes[TelemetryAttributes.httpHost.name] = parsedUrl.hostname; - attributes[TelemetryAttributes.urlScheme.name] = parsedUrl.protocol; - attributes[TelemetryAttributes.urlFull.name] = url; + attributes[TelemetryAttribute.HttpHost] = parsedUrl.hostname; + attributes[TelemetryAttribute.UrlScheme] = parsedUrl.protocol; + attributes[TelemetryAttribute.UrlFull] = url; } - if (start) attributes[TelemetryAttributes.httpClientRequestDuration.name] = Date.now() - start; - if (resendCount) attributes[TelemetryAttributes.httpRequestResendCount.name] = resendCount; + if (start) attributes[TelemetryAttribute.HttpClientRequestDuration] = Date.now() - start; + if (resendCount) attributes[TelemetryAttribute.HttpRequestResendCount] = resendCount; if (credentials && credentials.method === 'client_credentials') { - attributes[TelemetryAttributes.fgaClientRequestClientId.name] = credentials.configuration.clientId; + attributes[TelemetryAttribute.FgaClientRequestClientId] = credentials.configuration.clientId; } return attributes; @@ -85,17 +86,17 @@ export class TelemetryAttributes { credentials?: any; attributes?: Record; }): Record { - if (response?.status) attributes[TelemetryAttributes.httpResponseStatusCode.name] = response.status; + if (response?.status) attributes[TelemetryAttribute.HttpResponseStatusCode] = response.status; const responseHeaders = response?.headers || {}; const responseModelId = responseHeaders['openfga-authorization-model-id']; const responseQueryDuration = responseHeaders['fga-query-duration-ms']; - if (responseModelId) attributes[TelemetryAttributes.fgaClientResponseModelId.name] = responseModelId; - if (responseQueryDuration) attributes[TelemetryAttributes.httpServerRequestDuration.name] = responseQueryDuration; + if (responseModelId) attributes[TelemetryAttribute.FgaClientResponseModelId] = responseModelId; + if (responseQueryDuration) attributes[TelemetryAttribute.HttpServerRequestDuration] = responseQueryDuration; if (credentials && credentials.method === 'client_credentials') { - attributes[TelemetryAttributes.fgaClientRequestClientId.name] = credentials.configuration.clientId; + attributes[TelemetryAttribute.FgaClientRequestClientId] = credentials.configuration.clientId; } return attributes; diff --git a/telemetry/configuration.ts b/telemetry/configuration.ts index 17a7b3e..987a758 100644 --- a/telemetry/configuration.ts +++ b/telemetry/configuration.ts @@ -1,43 +1,28 @@ -import { TelemetryAttributes, TelemetryAttribute } from './attributes'; +import { TelemetryAttribute } from './attributes'; export class TelemetryMetricConfiguration { constructor( public enabled: boolean = true, - public attrFgaClientRequestClientId: boolean = true, - public attrFgaClientRequestMethod: boolean = true, - public attrFgaClientRequestModelId: boolean = true, - public attrFgaClientRequestStoreId: boolean = true, - public attrFgaClientResponseModelId: boolean = true, - public attrFgaClientUser: boolean = false, - public attrHttpClientRequestDuration: boolean = false, - public attrHttpHost: boolean = true, - public attrHttpRequestMethod: boolean = true, - public attrHttpRequestResendCount: boolean = true, - public attrHttpResponseStatusCode: boolean = true, - public attrHttpServerRequestDuration: boolean = false, - public attrUrlScheme: boolean = true, - public attrUrlFull: boolean = true, - public attrUserAgentOriginal: boolean = true + public attributes: Set = new Set([ + TelemetryAttribute.HttpHost, + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UserAgentOriginal, + TelemetryAttribute.FgaClientRequestMethod, + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.FgaClientRequestStoreId, + TelemetryAttribute.FgaClientRequestModelId, + TelemetryAttribute.HttpRequestResendCount, + TelemetryAttribute.FgaClientResponseModelId, + + // These metrics are not included by default because they are usually less useful + // TelemetryAttribute.UrlScheme, + // TelemetryAttribute.HttpRequestMethod, + // TelemetryAttribute.UrlFull, + + // This not included by default as it has a very high cardinality which could increase costs for users + // TelemetryAttribute.FgaClientUser + ]) ) {} - - attributes(): Record { - const enabled: Record = {}; - - Object.entries(this).forEach(([key, value]) => { - if (key.startsWith('attr') && value) { - let attrKey = key.replace('attr', '') as keyof typeof TelemetryAttributes; - attrKey = attrKey.charAt(0).toLowerCase() + attrKey.slice(1); - - const telemetryAttribute = TelemetryAttributes[attrKey as keyof typeof TelemetryAttributes] as TelemetryAttribute; - - if (telemetryAttribute) { - enabled[telemetryAttribute.name] = true; - } - } - }); - - return enabled; - } } export class TelemetryMetricsConfiguration { diff --git a/tests/telemetry/attributes.test.ts b/tests/telemetry/attributes.test.ts index 02dcc43..c51449c 100644 --- a/tests/telemetry/attributes.test.ts +++ b/tests/telemetry/attributes.test.ts @@ -1,4 +1,4 @@ -import { TelemetryAttributes } from '../../telemetry/attributes'; +import { TelemetryAttribute, TelemetryAttributes } from '../../telemetry/attributes'; describe('TelemetryAttributes', () => { let telemetryAttributes: TelemetryAttributes; @@ -13,12 +13,38 @@ describe('TelemetryAttributes', () => { 'http.host': 'example.com', }; - const filter = ['fga-client.request.client_id']; + const filter = new Set([TelemetryAttribute.FgaClientRequestClientId]); const prepared = telemetryAttributes.prepare(attributes, filter); expect(prepared).toEqual({ 'fga-client.request.client_id': 'test-client-id' }); }); + test('should return an empty object when attributes is provided but filter is undefined', () => { + const attributes = { + [TelemetryAttribute.HttpHost]: 'example.com', + [TelemetryAttribute.HttpResponseStatusCode]: 200, + }; + expect(telemetryAttributes.prepare(attributes)).toEqual({}); + }); + + test('should return an empty object when filter is provided but attributes is undefined', () => { + const filter = new Set([ + TelemetryAttribute.HttpHost, + ]); + expect(telemetryAttributes.prepare(undefined, filter)).toEqual({}); + }); + + test('should return an empty object when none of the attributes are in the filter set', () => { + const attributes = { + [TelemetryAttribute.HttpHost]: 'example.com', + [TelemetryAttribute.HttpResponseStatusCode]: 200, + }; + const filter = new Set([ + TelemetryAttribute.UserAgentOriginal, + ]); + expect(telemetryAttributes.prepare(attributes, filter)).toEqual({}); + }); + test('should create attributes from request correctly', () => { const result = telemetryAttributes.fromRequest({ userAgent: 'Mozilla/5.0', diff --git a/tests/telemetry/configuration.test.ts b/tests/telemetry/configuration.test.ts index d368490..e4a9b3d 100644 --- a/tests/telemetry/configuration.test.ts +++ b/tests/telemetry/configuration.test.ts @@ -1,51 +1,47 @@ import { TelemetryMetricConfiguration, TelemetryConfiguration, TelemetryMetricsConfiguration } from '../../telemetry/configuration'; -import { TelemetryAttributes } from '../../telemetry/attributes'; +import { TelemetryAttribute } from '../../telemetry/attributes'; describe('TelemetryMetricConfiguration', () => { test('should create a default TelemetryMetricConfiguration instance', () => { const config = new TelemetryMetricConfiguration(); expect(config.enabled).toBe(true); - expect(config.attrFgaClientRequestClientId).toBe(true); - expect(config.attrFgaClientRequestMethod).toBe(true); - expect(config.attrFgaClientRequestModelId).toBe(true); - expect(config.attrFgaClientRequestStoreId).toBe(true); - expect(config.attrFgaClientResponseModelId).toBe(true); - expect(config.attrFgaClientUser).toBe(false); + expect(config.attributes.has(TelemetryAttribute.HttpHost)); + expect(config.attributes.has(TelemetryAttribute.HttpResponseStatusCode)); + expect(config.attributes.has(TelemetryAttribute.UserAgentOriginal)); + expect(config.attributes.has(TelemetryAttribute.HttpRequestMethod)); + expect(config.attributes.has(TelemetryAttribute.FgaClientRequestMethod)); + expect(config.attributes.has(TelemetryAttribute.FgaClientRequestClientId)); + expect(config.attributes.has(TelemetryAttribute.FgaClientRequestStoreId)); + expect(config.attributes.has(TelemetryAttribute.FgaClientRequestModelId)); + expect(config.attributes.has(TelemetryAttribute.HttpRequestResendCount)); + expect(config.attributes.has(TelemetryAttribute.FgaClientResponseModelId)); + + // should not be there + expect(config.attributes.has(TelemetryAttribute.UrlScheme)).toBe(false); + expect(config.attributes.has(TelemetryAttribute.HttpRequestMethod)).toBe(false); + expect(config.attributes.has(TelemetryAttribute.UrlFull)).toBe(false); + expect(config.attributes.has(TelemetryAttribute.FgaClientUser)).toBe(false); }); test('should return correct attributes based on enabled properties', () => { const config = new TelemetryMetricConfiguration( true, // enabled - true, // attrFgaClientRequestClientId - false, // attrFgaClientRequestMethod - true, // attrFgaClientRequestModelId - false, // attrFgaClientRequestStoreId - true, // attrFgaClientResponseModelId - false, // attrFgaClientUser - true, // attrHttpClientRequestDuration - false, // attrHttpHost - true, // attrHttpRequestMethod - true, // attrHttpRequestResendCount - false, // attrHttpResponseStatusCode - true, // attrHttpServerRequestDuration - false, // attrUrlScheme - true, // attrUrlFull - false, // attrUserAgentOriginal + new Set([ + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UrlScheme, + TelemetryAttribute.HttpRequestMethod, + ]) ); - const attributes = config.attributes(); + const attributes = config.attributes; - expect(attributes).toEqual({ - [TelemetryAttributes.fgaClientRequestClientId.name]: true, - [TelemetryAttributes.fgaClientRequestModelId.name]: true, - [TelemetryAttributes.fgaClientResponseModelId.name]: true, - [TelemetryAttributes.httpClientRequestDuration.name]: true, - [TelemetryAttributes.httpRequestMethod.name]: true, - [TelemetryAttributes.httpRequestResendCount.name]: true, - [TelemetryAttributes.httpServerRequestDuration.name]: true, - [TelemetryAttributes.urlFull.name]: true, - }); + expect(attributes.size).toBe(4); + expect(attributes.has(TelemetryAttribute.FgaClientRequestClientId)); + expect(attributes.has(TelemetryAttribute.HttpResponseStatusCode)); + expect(attributes.has(TelemetryAttribute.UrlScheme)); + expect(attributes.has(TelemetryAttribute.HttpRequestMethod)); }); }); From e2d87ed461354d853af7106c4b2832d3ccd1cec1 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Fri, 13 Sep 2024 13:01:34 -0500 Subject: [PATCH 03/30] updates from running lint:fix --- configuration.ts | 2 +- telemetry/attributes.ts | 10 ++-- telemetry/configuration.ts | 2 +- telemetry/counters.ts | 6 +-- telemetry/histograms.ts | 12 ++--- telemetry/metrics.ts | 12 ++--- tests/telemetry/attributes.test.ts | 72 +++++++++++++-------------- tests/telemetry/configuration.test.ts | 16 +++--- tests/telemetry/counters.test.ts | 12 ++--- tests/telemetry/histograms.test.ts | 20 ++++---- tests/telemetry/metrics.test.ts | 20 ++++---- 11 files changed, 92 insertions(+), 92 deletions(-) diff --git a/configuration.ts b/configuration.ts index 6e70aaa..c4eb08b 100644 --- a/configuration.ts +++ b/configuration.ts @@ -14,7 +14,7 @@ import { ApiTokenConfig, AuthCredentialsConfig, ClientCredentialsConfig, CredentialsMethod } from "./credentials/types"; import { FgaValidationError, } from "./errors"; import { assertParamExists, isWellFormedUlidString, isWellFormedUriString } from "./validation"; -import { TelemetryConfiguration } from './telemetry/configuration'; +import { TelemetryConfiguration } from "./telemetry/configuration"; // default maximum number of retry const DEFAULT_MAX_RETRY = 15; diff --git a/telemetry/attributes.ts b/telemetry/attributes.ts index 2b71518..e4a2a03 100644 --- a/telemetry/attributes.ts +++ b/telemetry/attributes.ts @@ -1,4 +1,4 @@ -import { URL } from 'url'; +import { URL } from "url"; export enum TelemetryAttribute { FgaClientRequestClientId = "fga-client.request.client_id", @@ -70,7 +70,7 @@ export class TelemetryAttributes { if (start) attributes[TelemetryAttribute.HttpClientRequestDuration] = Date.now() - start; if (resendCount) attributes[TelemetryAttribute.HttpRequestResendCount] = resendCount; - if (credentials && credentials.method === 'client_credentials') { + if (credentials && credentials.method === "client_credentials") { attributes[TelemetryAttribute.FgaClientRequestClientId] = credentials.configuration.clientId; } @@ -89,13 +89,13 @@ export class TelemetryAttributes { if (response?.status) attributes[TelemetryAttribute.HttpResponseStatusCode] = response.status; const responseHeaders = response?.headers || {}; - const responseModelId = responseHeaders['openfga-authorization-model-id']; - const responseQueryDuration = responseHeaders['fga-query-duration-ms']; + const responseModelId = responseHeaders["openfga-authorization-model-id"]; + const responseQueryDuration = responseHeaders["fga-query-duration-ms"]; if (responseModelId) attributes[TelemetryAttribute.FgaClientResponseModelId] = responseModelId; if (responseQueryDuration) attributes[TelemetryAttribute.HttpServerRequestDuration] = responseQueryDuration; - if (credentials && credentials.method === 'client_credentials') { + if (credentials && credentials.method === "client_credentials") { attributes[TelemetryAttribute.FgaClientRequestClientId] = credentials.configuration.clientId; } diff --git a/telemetry/configuration.ts b/telemetry/configuration.ts index 987a758..baa3758 100644 --- a/telemetry/configuration.ts +++ b/telemetry/configuration.ts @@ -1,4 +1,4 @@ -import { TelemetryAttribute } from './attributes'; +import { TelemetryAttribute } from "./attributes"; export class TelemetryMetricConfiguration { constructor( diff --git a/telemetry/counters.ts b/telemetry/counters.ts index 3ed6cd8..a8f21f8 100644 --- a/telemetry/counters.ts +++ b/telemetry/counters.ts @@ -6,8 +6,8 @@ export interface TelemetryCounter { export class TelemetryCounters { static credentialsRequest: TelemetryCounter = { - name: 'fga-client.credentials.request', - unit: 'milliseconds', - description: 'The number of times an access token is requested.', + name: "fga-client.credentials.request", + unit: "milliseconds", + description: "The number of times an access token is requested.", }; } diff --git a/telemetry/histograms.ts b/telemetry/histograms.ts index 50d4558..3906a48 100644 --- a/telemetry/histograms.ts +++ b/telemetry/histograms.ts @@ -6,14 +6,14 @@ export interface TelemetryHistogram { export class TelemetryHistograms { static requestDuration: TelemetryHistogram = { - name: 'fga-client.request.duration', - unit: 'milliseconds', - description: 'How long it took for a request to be fulfilled.', + name: "fga-client.request.duration", + unit: "milliseconds", + description: "How long it took for a request to be fulfilled.", }; static queryDuration: TelemetryHistogram = { - name: 'fga-client.query.duration', - unit: 'milliseconds', - description: 'How long it took to perform a query request.', + name: "fga-client.query.duration", + unit: "milliseconds", + description: "How long it took to perform a query request.", }; } diff --git a/telemetry/metrics.ts b/telemetry/metrics.ts index 2334eb5..55cf758 100644 --- a/telemetry/metrics.ts +++ b/telemetry/metrics.ts @@ -1,8 +1,8 @@ -import { Counter, Histogram, Meter, ValueType } from '@opentelemetry/api'; -import { TelemetryConfiguration, TelemetryMetricsConfiguration } from './configuration'; -import { TelemetryCounters, TelemetryCounter } from './counters'; -import { TelemetryHistograms, TelemetryHistogram } from './histograms'; -import { TelemetryAttributes, TelemetryAttribute } from './attributes'; +import { Counter, Histogram, Meter, ValueType } from "@opentelemetry/api"; +import { TelemetryConfiguration, TelemetryMetricsConfiguration } from "./configuration"; +import { TelemetryCounters, TelemetryCounter } from "./counters"; +import { TelemetryHistograms, TelemetryHistogram } from "./histograms"; +import { TelemetryAttributes, TelemetryAttribute } from "./attributes"; import { metrics } from "@opentelemetry/api"; export class TelemetryMetrics { @@ -22,7 +22,7 @@ export class TelemetryMetrics { meter(): Meter { if (!this._meter) { - this._meter = metrics.getMeter("@openfga/sdk", "0.6.3") + this._meter = metrics.getMeter("@openfga/sdk", "0.6.3"); } return this._meter; } diff --git a/tests/telemetry/attributes.test.ts b/tests/telemetry/attributes.test.ts index c51449c..d5dad99 100644 --- a/tests/telemetry/attributes.test.ts +++ b/tests/telemetry/attributes.test.ts @@ -1,42 +1,42 @@ -import { TelemetryAttribute, TelemetryAttributes } from '../../telemetry/attributes'; +import { TelemetryAttribute, TelemetryAttributes } from "../../telemetry/attributes"; -describe('TelemetryAttributes', () => { +describe("TelemetryAttributes", () => { let telemetryAttributes: TelemetryAttributes; beforeEach(() => { telemetryAttributes = new TelemetryAttributes(); }); - test('should prepare attributes correctly', () => { + test("should prepare attributes correctly", () => { const attributes = { - 'fga-client.request.client_id': 'test-client-id', - 'http.host': 'example.com', + "fga-client.request.client_id": "test-client-id", + "http.host": "example.com", }; const filter = new Set([TelemetryAttribute.FgaClientRequestClientId]); const prepared = telemetryAttributes.prepare(attributes, filter); - expect(prepared).toEqual({ 'fga-client.request.client_id': 'test-client-id' }); + expect(prepared).toEqual({ "fga-client.request.client_id": "test-client-id" }); }); - test('should return an empty object when attributes is provided but filter is undefined', () => { + test("should return an empty object when attributes is provided but filter is undefined", () => { const attributes = { - [TelemetryAttribute.HttpHost]: 'example.com', + [TelemetryAttribute.HttpHost]: "example.com", [TelemetryAttribute.HttpResponseStatusCode]: 200, }; expect(telemetryAttributes.prepare(attributes)).toEqual({}); }); - test('should return an empty object when filter is provided but attributes is undefined', () => { + test("should return an empty object when filter is provided but attributes is undefined", () => { const filter = new Set([ TelemetryAttribute.HttpHost, ]); expect(telemetryAttributes.prepare(undefined, filter)).toEqual({}); }); - test('should return an empty object when none of the attributes are in the filter set', () => { + test("should return an empty object when none of the attributes are in the filter set", () => { const attributes = { - [TelemetryAttribute.HttpHost]: 'example.com', + [TelemetryAttribute.HttpHost]: "example.com", [TelemetryAttribute.HttpResponseStatusCode]: 200, }; const filter = new Set([ @@ -45,51 +45,51 @@ describe('TelemetryAttributes', () => { expect(telemetryAttributes.prepare(attributes, filter)).toEqual({}); }); - test('should create attributes from request correctly', () => { + test("should create attributes from request correctly", () => { const result = telemetryAttributes.fromRequest({ - userAgent: 'Mozilla/5.0', - fgaMethod: 'GET', - httpMethod: 'POST', - url: 'https://example.com', + userAgent: "Mozilla/5.0", + fgaMethod: "GET", + httpMethod: "POST", + url: "https://example.com", resendCount: 2, start: 1000, - credentials: { method: 'client_credentials', configuration: { clientId: 'client-id' } }, + credentials: { method: "client_credentials", configuration: { clientId: "client-id" } }, }); - expect(result['user_agent.original']).toEqual('Mozilla/5.0'); - expect(result['fga-client.request.method']).toEqual('GET'); - expect(result['http.request.method']).toEqual('POST'); - expect(result['http.host']).toEqual('example.com'); - expect(result['url.scheme']).toEqual('https:'); + expect(result["user_agent.original"]).toEqual("Mozilla/5.0"); + expect(result["fga-client.request.method"]).toEqual("GET"); + expect(result["http.request.method"]).toEqual("POST"); + expect(result["http.host"]).toEqual("example.com"); + expect(result["url.scheme"]).toEqual("https:"); }); - test('should create attributes from response correctly', () => { - const response = { status: 200, headers: { 'openfga-authorization-model-id': 'model-id', 'fga-query-duration-ms': '10' } }; + test("should create attributes from response correctly", () => { + const response = { status: 200, headers: { "openfga-authorization-model-id": "model-id", "fga-query-duration-ms": "10" } }; const result = telemetryAttributes.fromResponse({ response }); // Verify line 90 is covered - status is correctly set - expect(result['http.response.status_code']).toEqual(200); - expect(result['fga-client.response.model_id']).toEqual('model-id'); - expect(result['http.server.request.duration']).toEqual('10'); + expect(result["http.response.status_code"]).toEqual(200); + expect(result["fga-client.response.model_id"]).toEqual("model-id"); + expect(result["http.server.request.duration"]).toEqual("10"); }); - test('should handle response without status correctly', () => { - const response = { headers: { 'openfga-authorization-model-id': 'model-id', 'fga-query-duration-ms': '10' } }; + test("should handle response without status correctly", () => { + const response = { headers: { "openfga-authorization-model-id": "model-id", "fga-query-duration-ms": "10" } }; const result = telemetryAttributes.fromResponse({ response }); // Verify that no status code is set when response does not have a status - expect(result['http.response.status_code']).toBeUndefined(); - expect(result['fga-client.response.model_id']).toEqual('model-id'); - expect(result['http.server.request.duration']).toEqual('10'); + expect(result["http.response.status_code"]).toBeUndefined(); + expect(result["fga-client.response.model_id"]).toEqual("model-id"); + expect(result["http.server.request.duration"]).toEqual("10"); }); - test('should create attributes from response with client credentials', () => { + test("should create attributes from response with client credentials", () => { const response = { status: 200, headers: {} }; - const credentials = { method: 'client_credentials', configuration: { clientId: 'client-id' } }; + const credentials = { method: "client_credentials", configuration: { clientId: "client-id" } }; const result = telemetryAttributes.fromResponse({ response, credentials }); // Check that the client ID is set correctly from the credentials - expect(result['http.response.status_code']).toEqual(200); - expect(result['fga-client.request.client_id']).toEqual('client-id'); + expect(result["http.response.status_code"]).toEqual(200); + expect(result["fga-client.request.client_id"]).toEqual("client-id"); }); }); diff --git a/tests/telemetry/configuration.test.ts b/tests/telemetry/configuration.test.ts index e4a9b3d..d091c11 100644 --- a/tests/telemetry/configuration.test.ts +++ b/tests/telemetry/configuration.test.ts @@ -1,8 +1,8 @@ -import { TelemetryMetricConfiguration, TelemetryConfiguration, TelemetryMetricsConfiguration } from '../../telemetry/configuration'; -import { TelemetryAttribute } from '../../telemetry/attributes'; +import { TelemetryMetricConfiguration, TelemetryConfiguration, TelemetryMetricsConfiguration } from "../../telemetry/configuration"; +import { TelemetryAttribute } from "../../telemetry/attributes"; -describe('TelemetryMetricConfiguration', () => { - test('should create a default TelemetryMetricConfiguration instance', () => { +describe("TelemetryMetricConfiguration", () => { + test("should create a default TelemetryMetricConfiguration instance", () => { const config = new TelemetryMetricConfiguration(); expect(config.enabled).toBe(true); @@ -24,7 +24,7 @@ describe('TelemetryMetricConfiguration', () => { expect(config.attributes.has(TelemetryAttribute.FgaClientUser)).toBe(false); }); - test('should return correct attributes based on enabled properties', () => { + test("should return correct attributes based on enabled properties", () => { const config = new TelemetryMetricConfiguration( true, // enabled new Set([ @@ -45,15 +45,15 @@ describe('TelemetryMetricConfiguration', () => { }); }); -describe('TelemetryConfiguration', () => { - test('should create a default TelemetryConfiguration instance', () => { +describe("TelemetryConfiguration", () => { + test("should create a default TelemetryConfiguration instance", () => { const config = new TelemetryConfiguration(); expect(config.metrics).toBeInstanceOf(TelemetryMetricsConfiguration); expect(config.metrics.counterCredentialsRequest.enabled).toBe(true); }); - test('should allow overriding telemetry configuration options', () => { + test("should allow overriding telemetry configuration options", () => { const customMetrics = new TelemetryMetricsConfiguration(); customMetrics.counterCredentialsRequest.enabled = false; diff --git a/tests/telemetry/counters.test.ts b/tests/telemetry/counters.test.ts index 45fad6f..64ddb57 100644 --- a/tests/telemetry/counters.test.ts +++ b/tests/telemetry/counters.test.ts @@ -1,9 +1,9 @@ -import { TelemetryCounters } from '../../telemetry/counters'; +import { TelemetryCounters } from "../../telemetry/counters"; -describe('TelemetryCounters', () => { - test('should have correct counter details', () => { - expect(TelemetryCounters.credentialsRequest.name).toBe('fga-client.credentials.request'); - expect(TelemetryCounters.credentialsRequest.unit).toBe('milliseconds'); - expect(TelemetryCounters.credentialsRequest.description).toBe('The number of times an access token is requested.'); +describe("TelemetryCounters", () => { + test("should have correct counter details", () => { + expect(TelemetryCounters.credentialsRequest.name).toBe("fga-client.credentials.request"); + expect(TelemetryCounters.credentialsRequest.unit).toBe("milliseconds"); + expect(TelemetryCounters.credentialsRequest.description).toBe("The number of times an access token is requested."); }); }); diff --git a/tests/telemetry/histograms.test.ts b/tests/telemetry/histograms.test.ts index f31d915..a62a9a2 100644 --- a/tests/telemetry/histograms.test.ts +++ b/tests/telemetry/histograms.test.ts @@ -1,15 +1,15 @@ -import { TelemetryHistograms } from '../../telemetry/histograms'; +import { TelemetryHistograms } from "../../telemetry/histograms"; -describe('TelemetryHistograms', () => { - test('should have correct histogram details for request duration', () => { - expect(TelemetryHistograms.requestDuration.name).toBe('fga-client.request.duration'); - expect(TelemetryHistograms.requestDuration.unit).toBe('milliseconds'); - expect(TelemetryHistograms.requestDuration.description).toBe('How long it took for a request to be fulfilled.'); +describe("TelemetryHistograms", () => { + test("should have correct histogram details for request duration", () => { + expect(TelemetryHistograms.requestDuration.name).toBe("fga-client.request.duration"); + expect(TelemetryHistograms.requestDuration.unit).toBe("milliseconds"); + expect(TelemetryHistograms.requestDuration.description).toBe("How long it took for a request to be fulfilled."); }); - test('should have correct histogram details for query duration', () => { - expect(TelemetryHistograms.queryDuration.name).toBe('fga-client.query.duration'); - expect(TelemetryHistograms.queryDuration.unit).toBe('milliseconds'); - expect(TelemetryHistograms.queryDuration.description).toBe('How long it took to perform a query request.'); + test("should have correct histogram details for query duration", () => { + expect(TelemetryHistograms.queryDuration.name).toBe("fga-client.query.duration"); + expect(TelemetryHistograms.queryDuration.unit).toBe("milliseconds"); + expect(TelemetryHistograms.queryDuration.description).toBe("How long it took to perform a query request."); }); }); diff --git a/tests/telemetry/metrics.test.ts b/tests/telemetry/metrics.test.ts index d49f37a..a5adbc0 100644 --- a/tests/telemetry/metrics.test.ts +++ b/tests/telemetry/metrics.test.ts @@ -1,9 +1,9 @@ -import { TelemetryMetrics } from '../../telemetry/metrics'; -import { TelemetryCounters } from '../../telemetry/counters'; -import { TelemetryHistograms } from '../../telemetry/histograms'; -import { TelemetryAttributes } from '../../telemetry/attributes'; +import { TelemetryMetrics } from "../../telemetry/metrics"; +import { TelemetryCounters } from "../../telemetry/counters"; +import { TelemetryHistograms } from "../../telemetry/histograms"; +import { TelemetryAttributes } from "../../telemetry/attributes"; -jest.mock('@opentelemetry/api', () => ({ +jest.mock("@opentelemetry/api", () => ({ metrics: { getMeter: jest.fn().mockReturnValue({ createCounter: jest.fn().mockReturnValue({ add: jest.fn() }), @@ -12,29 +12,29 @@ jest.mock('@opentelemetry/api', () => ({ }, })); -describe('TelemetryMetrics', () => { +describe("TelemetryMetrics", () => { let telemetryMetrics: TelemetryMetrics; beforeEach(() => { telemetryMetrics = new TelemetryMetrics(); }); - test('should create a counter and add a value', () => { + test("should create a counter and add a value", () => { const counter = telemetryMetrics.counter(TelemetryCounters.credentialsRequest, 5); expect(counter).toBeDefined(); expect(counter.add).toHaveBeenCalledWith(5, undefined); }); - test('should create a histogram and record a value', () => { + test("should create a histogram and record a value", () => { const histogram = telemetryMetrics.histogram(TelemetryHistograms.requestDuration, 200); expect(histogram).toBeDefined(); expect(histogram.record).toHaveBeenCalledWith(200, undefined); }); - test('should handle creating metrics with custom attributes', () => { - const attributes = new TelemetryAttributes().prepare({ 'http.host': 'example.com' }); + test("should handle creating metrics with custom attributes", () => { + const attributes = new TelemetryAttributes().prepare({ "http.host": "example.com" }); const counter = telemetryMetrics.counter(TelemetryCounters.credentialsRequest, 3, attributes); expect(counter.add).toHaveBeenCalledWith(3, attributes); From 59556e557a97d6cd0b628c06928eefa7d6c88093 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 16 Sep 2024 16:27:12 -0500 Subject: [PATCH 04/30] remove enabled from config --- telemetry/configuration.ts | 3 ++- tests/telemetry/configuration.test.ts | 12 ------------ 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/telemetry/configuration.ts b/telemetry/configuration.ts index baa3758..535a409 100644 --- a/telemetry/configuration.ts +++ b/telemetry/configuration.ts @@ -2,7 +2,6 @@ import { TelemetryAttribute } from "./attributes"; export class TelemetryMetricConfiguration { constructor( - public enabled: boolean = true, public attributes: Set = new Set([ TelemetryAttribute.HttpHost, TelemetryAttribute.HttpResponseStatusCode, @@ -18,6 +17,8 @@ export class TelemetryMetricConfiguration { // TelemetryAttribute.UrlScheme, // TelemetryAttribute.HttpRequestMethod, // TelemetryAttribute.UrlFull, + // TelemetryAttribute.HttpClientRequestDuration, + // TelemetryAttribute.HttpServerRequestDuration // This not included by default as it has a very high cardinality which could increase costs for users // TelemetryAttribute.FgaClientUser diff --git a/tests/telemetry/configuration.test.ts b/tests/telemetry/configuration.test.ts index d091c11..c2058c6 100644 --- a/tests/telemetry/configuration.test.ts +++ b/tests/telemetry/configuration.test.ts @@ -5,7 +5,6 @@ describe("TelemetryMetricConfiguration", () => { test("should create a default TelemetryMetricConfiguration instance", () => { const config = new TelemetryMetricConfiguration(); - expect(config.enabled).toBe(true); expect(config.attributes.has(TelemetryAttribute.HttpHost)); expect(config.attributes.has(TelemetryAttribute.HttpResponseStatusCode)); expect(config.attributes.has(TelemetryAttribute.UserAgentOriginal)); @@ -26,7 +25,6 @@ describe("TelemetryMetricConfiguration", () => { test("should return correct attributes based on enabled properties", () => { const config = new TelemetryMetricConfiguration( - true, // enabled new Set([ TelemetryAttribute.FgaClientRequestClientId, TelemetryAttribute.HttpResponseStatusCode, @@ -50,15 +48,5 @@ describe("TelemetryConfiguration", () => { const config = new TelemetryConfiguration(); expect(config.metrics).toBeInstanceOf(TelemetryMetricsConfiguration); - expect(config.metrics.counterCredentialsRequest.enabled).toBe(true); - }); - - test("should allow overriding telemetry configuration options", () => { - const customMetrics = new TelemetryMetricsConfiguration(); - customMetrics.counterCredentialsRequest.enabled = false; - - const config = new TelemetryConfiguration(customMetrics); - - expect(config.metrics.counterCredentialsRequest.enabled).toBe(false); }); }); From f80acaaf92fb51ad2dfc3356c3146cd2427fb3ac Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 16 Sep 2024 16:27:38 -0500 Subject: [PATCH 05/30] don't send histogram unless configured to do so --- common.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common.ts b/common.ts index fdf2566..2dcee58 100644 --- a/common.ts +++ b/common.ts @@ -226,7 +226,7 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst attributes, }); - if (attributes[TelemetryAttribute.HttpServerRequestDuration]) { + if (configuration.telemetryConfig?.metrics?.histogramQueryDuration?.attributes?.has(TelemetryAttribute.HttpServerRequestDuration)) { telemetryMetrics.histogram( TelemetryHistograms.queryDuration, parseInt(attributes[TelemetryAttribute.HttpServerRequestDuration] as string, 10), @@ -237,7 +237,7 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst ); } - if (attributes[TelemetryAttribute.HttpClientRequestDuration]) { + if (configuration.telemetryConfig?.metrics?.histogramQueryDuration?.attributes?.has(TelemetryAttribute.HttpClientRequestDuration)) { telemetryMetrics.histogram( TelemetryHistograms.requestDuration, Date.now() - start, From 3be127db17de66e32e8fe9358441a3c8cf820001 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 16 Sep 2024 16:34:01 -0500 Subject: [PATCH 06/30] rename telemetryConfig to telemetry (config.telemetry instead of config.telemetryConfig) --- common.ts | 8 ++++---- configuration.ts | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/common.ts b/common.ts index 2dcee58..e2e42c5 100644 --- a/common.ts +++ b/common.ts @@ -226,24 +226,24 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst attributes, }); - if (configuration.telemetryConfig?.metrics?.histogramQueryDuration?.attributes?.has(TelemetryAttribute.HttpServerRequestDuration)) { + if (configuration.telemetry?.metrics?.histogramQueryDuration?.attributes?.has(TelemetryAttribute.HttpServerRequestDuration)) { telemetryMetrics.histogram( TelemetryHistograms.queryDuration, parseInt(attributes[TelemetryAttribute.HttpServerRequestDuration] as string, 10), telemetryAttributes.prepare( attributes, - configuration.telemetryConfig.metrics.histogramQueryDuration.attributes + configuration.telemetry.metrics.histogramQueryDuration.attributes ) ); } - if (configuration.telemetryConfig?.metrics?.histogramQueryDuration?.attributes?.has(TelemetryAttribute.HttpClientRequestDuration)) { + if (configuration.telemetry?.metrics?.histogramQueryDuration?.attributes?.has(TelemetryAttribute.HttpClientRequestDuration)) { telemetryMetrics.histogram( TelemetryHistograms.requestDuration, Date.now() - start, telemetryAttributes.prepare( attributes, - configuration.telemetryConfig.metrics.histogramRequestDuration.attributes + configuration.telemetry.metrics.histogramRequestDuration.attributes ) ); } diff --git a/configuration.ts b/configuration.ts index c4eb08b..cfc0d5f 100644 --- a/configuration.ts +++ b/configuration.ts @@ -42,7 +42,7 @@ export interface UserConfigurationParams { credentials?: CredentialsConfig; baseOptions?: any; retryParams?: RetryParams; - telemetryConfig?: TelemetryConfiguration; + telemetry?: TelemetryConfiguration; } export function GetDefaultRetryParams (maxRetry = DEFAULT_MAX_RETRY, minWaitInMs = DEFAULT_MIN_WAIT_MS) { @@ -128,7 +128,7 @@ export class Configuration { * @type {TelemetryConfiguration} * @memberof Configuration */ - telemetryConfig: TelemetryConfiguration; + telemetry: TelemetryConfiguration; constructor(params: UserConfigurationParams = {} as unknown as UserConfigurationParams) { this.apiScheme = params.apiScheme || this.apiScheme; @@ -177,7 +177,7 @@ export class Configuration { this.baseOptions = baseOptions; this.retryParams = params.retryParams; - this.telemetryConfig = params.telemetryConfig || new TelemetryConfiguration(); + this.telemetry = params.telemetry || new TelemetryConfiguration(); } /** From 941d227f2e579e9600948a599645f6e025672bef Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 17 Sep 2024 12:20:04 -0500 Subject: [PATCH 07/30] wip - attribute validation --- configuration.ts | 28 +++++++++++++++++++++++- telemetry/configuration.ts | 44 ++++++++++++++++++++++++++++++++++++++ tests/index.test.ts | 34 +++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 1 deletion(-) diff --git a/configuration.ts b/configuration.ts index cfc0d5f..2ed13f9 100644 --- a/configuration.ts +++ b/configuration.ts @@ -14,7 +14,8 @@ import { ApiTokenConfig, AuthCredentialsConfig, ClientCredentialsConfig, CredentialsMethod } from "./credentials/types"; import { FgaValidationError, } from "./errors"; import { assertParamExists, isWellFormedUlidString, isWellFormedUriString } from "./validation"; -import { TelemetryConfiguration } from "./telemetry/configuration"; +import { TelemetryConfiguration, validAttributes } from "./telemetry/configuration"; +import { TelemetryAttribute } from "./telemetry/attributes"; // default maximum number of retry const DEFAULT_MAX_RETRY = 15; @@ -203,6 +204,31 @@ export class Configuration { throw new FgaValidationError("Configuration.retryParams.maxRetry exceeds maximum allowed limit of 15"); } + if (this.telemetry?.metrics) { + const validAttrs = validAttributes(); + + const counterConfigAttrs = this.telemetry.metrics.counterCredentialsRequest?.attributes || new Set; + for (let counterConfigAttr in counterConfigAttrs) { + if (!validAttrs.has(counterConfigAttr as TelemetryAttribute)) { + throw new FgaValidationError(`Configuration.telemtry.metrics.counterCredentialsRequest attribute ${counterConfigAttr} is not a valid attribute`); + } + } + + const histogramRequestDurationConfigAttrs = this.telemetry.metrics.histogramRequestDuration?.attributes || new Set; + for (let histogramRequestDurationAttr in histogramRequestDurationConfigAttrs) { + if (!validAttrs.has(histogramRequestDurationAttr as TelemetryAttribute)) { + throw new FgaValidationError(`Configuration.telemtry.metrics.histogramRequestDuration attribute ${histogramRequestDurationAttr} is not a valid attribute`); + } + } + + const histogramQueryDurationConfigAttrs = this.telemetry.metrics.histogramQueryDuration?.attributes || new Set; + for (let histogramQueryDurationConfigAttr in histogramQueryDurationConfigAttrs) { + if (!validAttrs.has(histogramQueryDurationConfigAttr as TelemetryAttribute)) { + throw new FgaValidationError(`Configuration.telemtry.metrics.counterCredentialsRequest attribute ${histogramQueryDurationConfigAttrs} is not a valid attribute`); + } + } + } + return true; } diff --git a/telemetry/configuration.ts b/telemetry/configuration.ts index 535a409..1062087 100644 --- a/telemetry/configuration.ts +++ b/telemetry/configuration.ts @@ -36,4 +36,48 @@ export class TelemetryMetricsConfiguration { export class TelemetryConfiguration { constructor(public metrics: TelemetryMetricsConfiguration = new TelemetryMetricsConfiguration()) {} + // get isValid(): boolean { + // return true; + // // if (!this.metrics) { + // // return true; + // // } + // // return false; + // } } + +export function validAttributes(): Set { + return new Set([ + TelemetryAttribute.HttpHost, + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UserAgentOriginal, + TelemetryAttribute.FgaClientRequestMethod, + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.FgaClientRequestStoreId, + TelemetryAttribute.FgaClientRequestModelId, + TelemetryAttribute.HttpRequestResendCount, + TelemetryAttribute.FgaClientResponseModelId, + TelemetryAttribute.UrlScheme, + TelemetryAttribute.HttpRequestMethod, + TelemetryAttribute.UrlFull, + TelemetryAttribute.HttpClientRequestDuration, + TelemetryAttribute.HttpServerRequestDuration, + TelemetryAttribute.FgaClientUser + ]); +} + +// export function isValid(config: TelemetryConfiguration): boolean { +// if (!config.metrics) { +// return true; +// } + +// const validAttrs = validAttributes(); + +// const counterConfigAttrs = config.metrics.counterCredentialsRequest?.attributes; +// for (let counterConfigAttr in counterConfigAttrs) { +// if (!validAttrs.has(counterConfigAttr as TelemetryAttribute)) { +// return false; +// } +// } + +// return true; +// } \ No newline at end of file diff --git a/tests/index.test.ts b/tests/index.test.ts index 78de7e7..79c754b 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -25,6 +25,9 @@ import { FgaApiValidationError, FgaValidationError, OpenFgaApi, + TelemetryConfiguration, + TelemetryMetricConfiguration, + TelemetryMetricsConfiguration, } from "../index"; import { CallResult } from "../common"; import { GetDefaultRetryParams } from "../configuration"; @@ -35,6 +38,7 @@ import { OPENFGA_API_TOKEN_ISSUER, } from "./helpers/default-config"; import { getNocks } from "./helpers/nocks"; +import { TelemetryAttribute } from "../dist"; const nocks = getNocks(nock); nock.disableNetConnect(); @@ -249,6 +253,36 @@ describe("OpenFGA SDK", function () { const configuration = new Configuration(baseConfig); expect(() => new OpenFgaApi(configuration)).not.toThrowError(); }); + + it("should only accept valid telemetry attributes", async () => { + // const metricConfig = new TelemetryMetricConfiguration(new Set); + // const metricsConfig = new TelemetryMetricsConfiguration( + // metricConfig, + // metricConfig, + // metricConfig, + // ); + // const telConfig = new TelemetryConfiguration(metricsConfig); + expect( + () => + new OpenFgaApi({ + ...baseConfig, + telemetry: telConfig, + // telemetry: { + // metrics: { + // counterCredentialsRequest: { + // attributes: ["JUNK"] as any + // }, + // histogramQueryDuration: { + // attributes: new Set + // }, + // histogramRequestDuration: { + // attributes: new Set + // } + // } + // } + }) + ).toThrow(); + }); }); describe("error handling", () => { From ce4e721bcbbff0671290bacacf1ddb2e917968c1 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 17 Sep 2024 12:20:30 -0500 Subject: [PATCH 08/30] lint fixes --- configuration.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/configuration.ts b/configuration.ts index 2ed13f9..a4ebea0 100644 --- a/configuration.ts +++ b/configuration.ts @@ -208,22 +208,22 @@ export class Configuration { const validAttrs = validAttributes(); const counterConfigAttrs = this.telemetry.metrics.counterCredentialsRequest?.attributes || new Set; - for (let counterConfigAttr in counterConfigAttrs) { - if (!validAttrs.has(counterConfigAttr as TelemetryAttribute)) { + for (const counterConfigAttr in counterConfigAttrs) { + if (!validAttrs.has(counterConfigAttr as TelemetryAttribute)) { throw new FgaValidationError(`Configuration.telemtry.metrics.counterCredentialsRequest attribute ${counterConfigAttr} is not a valid attribute`); } } const histogramRequestDurationConfigAttrs = this.telemetry.metrics.histogramRequestDuration?.attributes || new Set; - for (let histogramRequestDurationAttr in histogramRequestDurationConfigAttrs) { - if (!validAttrs.has(histogramRequestDurationAttr as TelemetryAttribute)) { + for (const histogramRequestDurationAttr in histogramRequestDurationConfigAttrs) { + if (!validAttrs.has(histogramRequestDurationAttr as TelemetryAttribute)) { throw new FgaValidationError(`Configuration.telemtry.metrics.histogramRequestDuration attribute ${histogramRequestDurationAttr} is not a valid attribute`); } } const histogramQueryDurationConfigAttrs = this.telemetry.metrics.histogramQueryDuration?.attributes || new Set; - for (let histogramQueryDurationConfigAttr in histogramQueryDurationConfigAttrs) { - if (!validAttrs.has(histogramQueryDurationConfigAttr as TelemetryAttribute)) { + for (const histogramQueryDurationConfigAttr in histogramQueryDurationConfigAttrs) { + if (!validAttrs.has(histogramQueryDurationConfigAttr as TelemetryAttribute)) { throw new FgaValidationError(`Configuration.telemtry.metrics.counterCredentialsRequest attribute ${histogramQueryDurationConfigAttrs} is not a valid attribute`); } } From 87d6ecdd80394f9a2db5c652c871421642eb7ffd Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 17 Sep 2024 12:21:26 -0500 Subject: [PATCH 09/30] update example --- example/opentelemetry/opentelemetry.mjs | 48 +++++++++++++++++++++++-- example/opentelemetry/package.json | 2 +- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/example/opentelemetry/opentelemetry.mjs b/example/opentelemetry/opentelemetry.mjs index 63c82a1..745c357 100644 --- a/example/opentelemetry/opentelemetry.mjs +++ b/example/opentelemetry/opentelemetry.mjs @@ -1,5 +1,6 @@ import "dotenv/config"; -import { CredentialsMethod, FgaApiValidationError, OpenFgaClient } from "@openfga/sdk"; +import { CredentialsMethod, FgaApiValidationError, OpenFgaClient, TelemetryAttribute, TelemetryMetricConfiguration, TelemetryMetricsConfiguration } from "@openfga/sdk"; +// import { TelemetryConfiguration } from "../../telemetry/configuration"; let credentials; if (process.env.FGA_CLIENT_ID) { @@ -14,11 +15,54 @@ if (process.env.FGA_CLIENT_ID) { }; } +// define desired telemetry options +const counterCredentialsRequestAttributes = new Set([ + TelemetryAttribute.HttpHost, + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UserAgentOriginal, + TelemetryAttribute.FgaClientRequestClientId +]); + +const histogramRequestDurationAttributes = new Set([ + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UserAgentOriginal, + TelemetryAttribute.HttpRequestMethod, + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.FgaClientRequestStoreId, + TelemetryAttribute.FgaClientResponseModelId, + TelemetryAttribute.HttpRequestResendCount, +]); + +const histogramQueryDurationAttributes = new Set([ + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UserAgentOriginal, + TelemetryAttribute.HttpRequestMethod, + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.FgaClientRequestStoreId, + TelemetryAttribute.FgaClientResponseModelId, + TelemetryAttribute.HttpRequestResendCount, +]); + +const telemetryConfig = { + metrics: { + counterCredentialsRequest: { + attributes: counterCredentialsRequestAttributes + }, + histogramRequestDuration: { + attributes: histogramRequestDurationAttributes + }, + histogramQueryDuration: { + attributes: histogramQueryDurationAttributes + } + } +}; + const fgaClient = new OpenFgaClient({ apiUrl: process.env.FGA_API_URL, storeId: process.env.FGA_STORE_ID, authorizationModelId: process.env.FGA_MODEL_ID, - credentials + credentials, + telemetry: telemetryConfig, }); async function main () { diff --git a/example/opentelemetry/package.json b/example/opentelemetry/package.json index c204552..e6d743a 100644 --- a/example/opentelemetry/package.json +++ b/example/opentelemetry/package.json @@ -9,7 +9,7 @@ "start": "node --import ./instrumentation.mjs opentelemetry.mjs" }, "dependencies": { - "@openfga/sdk": "^0.6.3", + "@openfga/sdk": "file:../../", "@opentelemetry/exporter-metrics-otlp-proto": "^0.52.1", "@opentelemetry/exporter-prometheus": "^0.52.1", "@opentelemetry/sdk-metrics": "^1.25.1", From 759a38460975fccf2083fecc63bd626716c0a67f Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 17 Sep 2024 12:22:40 -0500 Subject: [PATCH 10/30] fix test --- tests/index.test.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/index.test.ts b/tests/index.test.ts index 79c754b..d6d927e 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -266,20 +266,20 @@ describe("OpenFGA SDK", function () { () => new OpenFgaApi({ ...baseConfig, - telemetry: telConfig, - // telemetry: { - // metrics: { - // counterCredentialsRequest: { - // attributes: ["JUNK"] as any - // }, - // histogramQueryDuration: { - // attributes: new Set - // }, - // histogramRequestDuration: { - // attributes: new Set - // } - // } - // } + // telemetry: telConfig, + telemetry: { + metrics: { + counterCredentialsRequest: { + attributes: ["JUNK"] as any + }, + histogramQueryDuration: { + attributes: new Set + }, + histogramRequestDuration: { + attributes: new Set + } + } + } }) ).toThrow(); }); From 336bac49efd32004760cdf0c9ff4705516d4f6b9 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 17 Sep 2024 13:17:35 -0500 Subject: [PATCH 11/30] fix bad import in test --- tests/index.test.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/index.test.ts b/tests/index.test.ts index d6d927e..a682a3d 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -25,6 +25,7 @@ import { FgaApiValidationError, FgaValidationError, OpenFgaApi, + TelemetryAttribute, TelemetryConfiguration, TelemetryMetricConfiguration, TelemetryMetricsConfiguration, @@ -38,7 +39,6 @@ import { OPENFGA_API_TOKEN_ISSUER, } from "./helpers/default-config"; import { getNocks } from "./helpers/nocks"; -import { TelemetryAttribute } from "../dist"; const nocks = getNocks(nock); nock.disableNetConnect(); @@ -262,10 +262,26 @@ describe("OpenFGA SDK", function () { // metricConfig, // ); // const telConfig = new TelemetryConfiguration(metricsConfig); + + // const telemetryConfig = { + // metrics: { + // counterCredentialsRequest: { + // attributes: new Set + // }, + // histogramRequestDuration: { + // attributes: new Set + // }, + // histogramQueryDuration: { + // attributes: new Set + // } + // } + // }; + expect( () => new OpenFgaApi({ ...baseConfig, + // telemetry: telemetryConfig, // telemetry: telConfig, telemetry: { metrics: { From b429bd519a418930a61593aab4f8819867ab034c Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 17 Sep 2024 14:26:03 -0500 Subject: [PATCH 12/30] only send telemetry for credentials when configured --- credentials/credentials.ts | 42 +++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/credentials/credentials.ts b/credentials/credentials.ts index 930992f..97d982f 100644 --- a/credentials/credentials.ts +++ b/credentials/credentials.ts @@ -16,20 +16,21 @@ import globalAxios, { AxiosInstance } from "axios"; import { assertParamExists, isWellFormedUriString } from "../validation"; import { FgaApiAuthenticationError, FgaApiError, FgaError, FgaValidationError } from "../errors"; import { attemptHttpRequest } from "../common"; -import { ApiTokenConfig, AuthCredentialsConfig, ClientCredentialsConfig, CredentialsMethod } from "./types"; +import { AuthCredentialsConfig, ClientCredentialsConfig, CredentialsMethod } from "./types"; import { TelemetryAttributes } from "../telemetry/attributes"; import { TelemetryMetrics } from "../telemetry/metrics"; import { TelemetryCounters } from "../telemetry/counters"; +import { TelemetryConfiguration } from "../telemetry/configuration"; export class Credentials { private accessToken?: string; private accessTokenExpiryDate?: Date; - public static init(configuration: { credentials: AuthCredentialsConfig }): Credentials { - return new Credentials(configuration.credentials); + public static init(configuration: { credentials: AuthCredentialsConfig, telemetry: TelemetryConfiguration }): Credentials { + return new Credentials(configuration.credentials, globalAxios, configuration.telemetry); } - public constructor(private authConfig: AuthCredentialsConfig, private axios: AxiosInstance = globalAxios) { + public constructor(private authConfig: AuthCredentialsConfig, private axios: AxiosInstance = globalAxios, private telemetryConfig: TelemetryConfiguration) { this.initConfig(); this.isValid(); } @@ -160,25 +161,28 @@ export class Credentials { this.accessTokenExpiryDate = new Date(Date.now() + response.data.expires_in * 1000); } - const telemetryAttributes = new TelemetryAttributes(); - const telemetryMetrics = new TelemetryMetrics(); + // TODO is this the right check? + if (this.telemetryConfig?.metrics?.counterCredentialsRequest?.attributes) { + const telemetryAttributes = new TelemetryAttributes(); + const telemetryMetrics = new TelemetryMetrics(); - let attributes = {}; + let attributes = {}; - attributes = telemetryAttributes.fromRequest({ - credentials: clientCredentials, - // resendCount: 0, // TODO: implement resend count tracking, not available in the current context - attributes, - }); + attributes = telemetryAttributes.fromRequest({ + credentials: clientCredentials, + // resendCount: 0, // TODO: implement resend count tracking, not available in the current context + attributes, + }); - attributes = telemetryAttributes.fromResponse({ - response, - credentials: clientCredentials, - attributes, - }); + attributes = telemetryAttributes.fromResponse({ + response, + credentials: clientCredentials, + attributes, + }); - attributes = telemetryAttributes.prepare(attributes); - telemetryMetrics.counter(TelemetryCounters.credentialsRequest, 1, attributes); + attributes = telemetryAttributes.prepare(attributes); + telemetryMetrics.counter(TelemetryCounters.credentialsRequest, 1, attributes); + } return this.accessToken; } catch (err: unknown) { From 5a2a9c5b8058efc0cafb2439d1d00b672d963c77 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 17 Sep 2024 14:35:58 -0500 Subject: [PATCH 13/30] cleanup --- telemetry/configuration.ts | 67 +++++++++++++++----------------------- 1 file changed, 27 insertions(+), 40 deletions(-) diff --git a/telemetry/configuration.ts b/telemetry/configuration.ts index 1062087..23f42ca 100644 --- a/telemetry/configuration.ts +++ b/telemetry/configuration.ts @@ -1,28 +1,30 @@ import { TelemetryAttribute } from "./attributes"; -export class TelemetryMetricConfiguration { - constructor( - public attributes: Set = new Set([ - TelemetryAttribute.HttpHost, - TelemetryAttribute.HttpResponseStatusCode, - TelemetryAttribute.UserAgentOriginal, - TelemetryAttribute.FgaClientRequestMethod, - TelemetryAttribute.FgaClientRequestClientId, - TelemetryAttribute.FgaClientRequestStoreId, - TelemetryAttribute.FgaClientRequestModelId, - TelemetryAttribute.HttpRequestResendCount, - TelemetryAttribute.FgaClientResponseModelId, +const defaultAttributes = new Set([ + TelemetryAttribute.HttpHost, + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UserAgentOriginal, + TelemetryAttribute.FgaClientRequestMethod, + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.FgaClientRequestStoreId, + TelemetryAttribute.FgaClientRequestModelId, + TelemetryAttribute.HttpRequestResendCount, + TelemetryAttribute.FgaClientResponseModelId, + + // These metrics are not included by default because they are usually less useful + // TelemetryAttribute.UrlScheme, + // TelemetryAttribute.HttpRequestMethod, + // TelemetryAttribute.UrlFull, + // TelemetryAttribute.HttpClientRequestDuration, + // TelemetryAttribute.HttpServerRequestDuration - // These metrics are not included by default because they are usually less useful - // TelemetryAttribute.UrlScheme, - // TelemetryAttribute.HttpRequestMethod, - // TelemetryAttribute.UrlFull, - // TelemetryAttribute.HttpClientRequestDuration, - // TelemetryAttribute.HttpServerRequestDuration + // This not included by default as it has a very high cardinality which could increase costs for users + // TelemetryAttribute.FgaClientUser +]); - // This not included by default as it has a very high cardinality which could increase costs for users - // TelemetryAttribute.FgaClientUser - ]) +export class TelemetryMetricConfiguration { + constructor( + public attributes: Set = defaultAttributes ) {} } @@ -36,12 +38,14 @@ export class TelemetryMetricsConfiguration { export class TelemetryConfiguration { constructor(public metrics: TelemetryMetricsConfiguration = new TelemetryMetricsConfiguration()) {} - // get isValid(): boolean { + + // TODO move validation to a method here, like this (causing usage issues currently): + // isValid(): boolean { // return true; // // if (!this.metrics) { // // return true; // // } - // // return false; + // ... // } } @@ -64,20 +68,3 @@ export function validAttributes(): Set { TelemetryAttribute.FgaClientUser ]); } - -// export function isValid(config: TelemetryConfiguration): boolean { -// if (!config.metrics) { -// return true; -// } - -// const validAttrs = validAttributes(); - -// const counterConfigAttrs = config.metrics.counterCredentialsRequest?.attributes; -// for (let counterConfigAttr in counterConfigAttrs) { -// if (!validAttrs.has(counterConfigAttr as TelemetryAttribute)) { -// return false; -// } -// } - -// return true; -// } \ No newline at end of file From f39a136b864b720c33b9bb02d7dc077164a6029d Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Tue, 17 Sep 2024 15:29:13 -0500 Subject: [PATCH 14/30] fix up telemetry validation --- configuration.ts | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/configuration.ts b/configuration.ts index a4ebea0..2b1644d 100644 --- a/configuration.ts +++ b/configuration.ts @@ -16,6 +16,7 @@ import { FgaValidationError, } from "./errors"; import { assertParamExists, isWellFormedUlidString, isWellFormedUriString } from "./validation"; import { TelemetryConfiguration, validAttributes } from "./telemetry/configuration"; import { TelemetryAttribute } from "./telemetry/attributes"; +import { count } from "console"; // default maximum number of retry const DEFAULT_MAX_RETRY = 15; @@ -208,25 +209,25 @@ export class Configuration { const validAttrs = validAttributes(); const counterConfigAttrs = this.telemetry.metrics.counterCredentialsRequest?.attributes || new Set; - for (const counterConfigAttr in counterConfigAttrs) { - if (!validAttrs.has(counterConfigAttr as TelemetryAttribute)) { - throw new FgaValidationError(`Configuration.telemtry.metrics.counterCredentialsRequest attribute ${counterConfigAttr} is not a valid attribute`); + counterConfigAttrs.forEach(counterConfigAttr => { + if (!validAttrs.has(counterConfigAttr)) { + throw new FgaValidationError(`Configuration.telemtry.metrics.counterCredentialsRequest attribute '${counterConfigAttrs}' is not a valid attribute`); } - } + }); const histogramRequestDurationConfigAttrs = this.telemetry.metrics.histogramRequestDuration?.attributes || new Set; - for (const histogramRequestDurationAttr in histogramRequestDurationConfigAttrs) { - if (!validAttrs.has(histogramRequestDurationAttr as TelemetryAttribute)) { - throw new FgaValidationError(`Configuration.telemtry.metrics.histogramRequestDuration attribute ${histogramRequestDurationAttr} is not a valid attribute`); + histogramRequestDurationConfigAttrs.forEach(histogramRequestDurationAttr => { + if (!validAttrs.has(histogramRequestDurationAttr)) { + throw new FgaValidationError(`Configuration.telemtry.metrics.histogramRequestDuration attribute '${histogramRequestDurationAttr}' is not a valid attribute`); } - } + }); const histogramQueryDurationConfigAttrs = this.telemetry.metrics.histogramQueryDuration?.attributes || new Set; - for (const histogramQueryDurationConfigAttr in histogramQueryDurationConfigAttrs) { - if (!validAttrs.has(histogramQueryDurationConfigAttr as TelemetryAttribute)) { - throw new FgaValidationError(`Configuration.telemtry.metrics.counterCredentialsRequest attribute ${histogramQueryDurationConfigAttrs} is not a valid attribute`); + histogramQueryDurationConfigAttrs.forEach(histogramQueryDurationConfigAttr => { + if (!validAttrs.has(histogramQueryDurationConfigAttr)) { + throw new FgaValidationError(`Configuration.telemtry.metrics.histogramQueryDuration attribute '${histogramQueryDurationConfigAttrs}' is not a valid attribute`); } - } + }); } return true; From 1d9ac73475eb64dcc65b92b0cfaff09819a61b69 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Wed, 18 Sep 2024 16:42:54 -0500 Subject: [PATCH 15/30] fix metrics reporting and object initialization --- common.ts | 4 +-- configuration.ts | 7 +++-- telemetry/configuration.ts | 64 +++++++++++++++++++++++++++++--------- 3 files changed, 55 insertions(+), 20 deletions(-) diff --git a/common.ts b/common.ts index e2e42c5..44432e3 100644 --- a/common.ts +++ b/common.ts @@ -226,7 +226,7 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst attributes, }); - if (configuration.telemetry?.metrics?.histogramQueryDuration?.attributes?.has(TelemetryAttribute.HttpServerRequestDuration)) { + if (configuration.telemetry?.metrics?.histogramQueryDuration) { telemetryMetrics.histogram( TelemetryHistograms.queryDuration, parseInt(attributes[TelemetryAttribute.HttpServerRequestDuration] as string, 10), @@ -237,7 +237,7 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst ); } - if (configuration.telemetry?.metrics?.histogramQueryDuration?.attributes?.has(TelemetryAttribute.HttpClientRequestDuration)) { + if (configuration.telemetry?.metrics?.histogramRequestDuration) { telemetryMetrics.histogram( TelemetryHistograms.requestDuration, Date.now() - start, diff --git a/configuration.ts b/configuration.ts index 2b1644d..caa4aa0 100644 --- a/configuration.ts +++ b/configuration.ts @@ -14,7 +14,7 @@ import { ApiTokenConfig, AuthCredentialsConfig, ClientCredentialsConfig, CredentialsMethod } from "./credentials/types"; import { FgaValidationError, } from "./errors"; import { assertParamExists, isWellFormedUlidString, isWellFormedUriString } from "./validation"; -import { TelemetryConfiguration, validAttributes } from "./telemetry/configuration"; +import { TelemetryConfig, TelemetryConfiguration, validAttributes } from "./telemetry/configuration"; import { TelemetryAttribute } from "./telemetry/attributes"; import { count } from "console"; @@ -44,7 +44,7 @@ export interface UserConfigurationParams { credentials?: CredentialsConfig; baseOptions?: any; retryParams?: RetryParams; - telemetry?: TelemetryConfiguration; + telemetry?: TelemetryConfig; } export function GetDefaultRetryParams (maxRetry = DEFAULT_MAX_RETRY, minWaitInMs = DEFAULT_MIN_WAIT_MS) { @@ -179,7 +179,8 @@ export class Configuration { this.baseOptions = baseOptions; this.retryParams = params.retryParams; - this.telemetry = params.telemetry || new TelemetryConfiguration(); + this.telemetry = new TelemetryConfiguration(params?.telemetry?.metrics); + // this.telemetry = params.telemetry || new TelemetryConfiguration(); } /** diff --git a/telemetry/configuration.ts b/telemetry/configuration.ts index 23f42ca..84c73c9 100644 --- a/telemetry/configuration.ts +++ b/telemetry/configuration.ts @@ -1,5 +1,6 @@ import { TelemetryAttribute } from "./attributes"; + const defaultAttributes = new Set([ TelemetryAttribute.HttpHost, TelemetryAttribute.HttpResponseStatusCode, @@ -22,31 +23,64 @@ const defaultAttributes = new Set([ // TelemetryAttribute.FgaClientUser ]); -export class TelemetryMetricConfiguration { +export class TelemetryMetricConfiguration implements TelemetryMetricConfig { constructor( public attributes: Set = defaultAttributes ) {} } -export class TelemetryMetricsConfiguration { +// Drop this whole class. Do this logic in the calling class (TelemetryCOnfiguration) +export class TelemetryMetricsConfiguration implements MetricsConfig { + // instead of taking 3 individual metrics, accept a map. e.g., + // ["counterCredentialsRequest": TelemetryMetricConfig] + // only constructor will change constructor( - public counterCredentialsRequest: TelemetryMetricConfiguration = new TelemetryMetricConfiguration(), - public histogramRequestDuration: TelemetryMetricConfiguration = new TelemetryMetricConfiguration(), - public histogramQueryDuration: TelemetryMetricConfiguration = new TelemetryMetricConfiguration() - ) {} + public counterCredentialsRequest: TelemetryMetricConfig = new TelemetryMetricConfiguration(), + public histogramRequestDuration: TelemetryMetricConfig = new TelemetryMetricConfiguration(), + public histogramQueryDuration: TelemetryMetricConfig = new TelemetryMetricConfiguration() + ) { + if (!(counterCredentialsRequest instanceof TelemetryMetricConfiguration)) { + counterCredentialsRequest = new TelemetryMetricConfiguration(counterCredentialsRequest.attributes); + } + if (!(histogramRequestDuration instanceof TelemetryMetricConfiguration)) { + histogramRequestDuration = new TelemetryMetricConfiguration(histogramRequestDuration.attributes); + } + if (!(histogramQueryDuration instanceof TelemetryMetricConfiguration)) { + histogramQueryDuration = new TelemetryMetricConfiguration(histogramQueryDuration.attributes); + } + } } -export class TelemetryConfiguration { - constructor(public metrics: TelemetryMetricsConfiguration = new TelemetryMetricsConfiguration()) {} +export interface TelemetryMetricConfig { + attributes: Set; +} + +export interface MetricsConfig { + counterCredentialsRequest: TelemetryMetricConfiguration; + histogramRequestDuration: TelemetryMetricConfiguration; + histogramQueryDuration: TelemetryMetricConfiguration; +} + +export interface TelemetryConfig { + metrics: MetricsConfig +} + +export class TelemetryConfiguration implements TelemetryConfig { + constructor(public metrics: MetricsConfig = new TelemetryMetricsConfiguration()) { + if (!(metrics instanceof TelemetryMetricsConfiguration)) { + // metrics = {"counterCredentialsRequest": new TelemetryMetricConfiguration(metrics.counterCredentialsRequest.attributes) } + metrics = new TelemetryMetricsConfiguration(metrics.counterCredentialsRequest, metrics.histogramRequestDuration, metrics.histogramQueryDuration); + } + } // TODO move validation to a method here, like this (causing usage issues currently): - // isValid(): boolean { - // return true; - // // if (!this.metrics) { - // // return true; - // // } - // ... - // } + isValid(): boolean { + return true; + // if (!this.metrics) { + // return true; + // } + + } } export function validAttributes(): Set { From b9460753d917691026e59d3ca11cad0df61d39f5 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Wed, 18 Sep 2024 22:18:08 -0500 Subject: [PATCH 16/30] move validation to telemetry config --- configuration.ts | 26 +------------------------- telemetry/configuration.ts | 31 ++++++++++++++++++++++++------- 2 files changed, 25 insertions(+), 32 deletions(-) diff --git a/configuration.ts b/configuration.ts index caa4aa0..019a026 100644 --- a/configuration.ts +++ b/configuration.ts @@ -180,7 +180,6 @@ export class Configuration { this.baseOptions = baseOptions; this.retryParams = params.retryParams; this.telemetry = new TelemetryConfiguration(params?.telemetry?.metrics); - // this.telemetry = params.telemetry || new TelemetryConfiguration(); } /** @@ -206,30 +205,7 @@ export class Configuration { throw new FgaValidationError("Configuration.retryParams.maxRetry exceeds maximum allowed limit of 15"); } - if (this.telemetry?.metrics) { - const validAttrs = validAttributes(); - - const counterConfigAttrs = this.telemetry.metrics.counterCredentialsRequest?.attributes || new Set; - counterConfigAttrs.forEach(counterConfigAttr => { - if (!validAttrs.has(counterConfigAttr)) { - throw new FgaValidationError(`Configuration.telemtry.metrics.counterCredentialsRequest attribute '${counterConfigAttrs}' is not a valid attribute`); - } - }); - - const histogramRequestDurationConfigAttrs = this.telemetry.metrics.histogramRequestDuration?.attributes || new Set; - histogramRequestDurationConfigAttrs.forEach(histogramRequestDurationAttr => { - if (!validAttrs.has(histogramRequestDurationAttr)) { - throw new FgaValidationError(`Configuration.telemtry.metrics.histogramRequestDuration attribute '${histogramRequestDurationAttr}' is not a valid attribute`); - } - }); - - const histogramQueryDurationConfigAttrs = this.telemetry.metrics.histogramQueryDuration?.attributes || new Set; - histogramQueryDurationConfigAttrs.forEach(histogramQueryDurationConfigAttr => { - if (!validAttrs.has(histogramQueryDurationConfigAttr)) { - throw new FgaValidationError(`Configuration.telemtry.metrics.histogramQueryDuration attribute '${histogramQueryDurationConfigAttrs}' is not a valid attribute`); - } - }); - } + this.telemetry.ensureValid(); return true; } diff --git a/telemetry/configuration.ts b/telemetry/configuration.ts index 84c73c9..8282222 100644 --- a/telemetry/configuration.ts +++ b/telemetry/configuration.ts @@ -1,3 +1,4 @@ +import { FgaValidationError } from "../errors"; import { TelemetryAttribute } from "./attributes"; @@ -68,17 +69,33 @@ export interface TelemetryConfig { export class TelemetryConfiguration implements TelemetryConfig { constructor(public metrics: MetricsConfig = new TelemetryMetricsConfiguration()) { if (!(metrics instanceof TelemetryMetricsConfiguration)) { - // metrics = {"counterCredentialsRequest": new TelemetryMetricConfiguration(metrics.counterCredentialsRequest.attributes) } metrics = new TelemetryMetricsConfiguration(metrics.counterCredentialsRequest, metrics.histogramRequestDuration, metrics.histogramQueryDuration); } } - // TODO move validation to a method here, like this (causing usage issues currently): - isValid(): boolean { - return true; - // if (!this.metrics) { - // return true; - // } + public ensureValid(): void { + const validAttrs = validAttributes(); + + const counterConfigAttrs = this.metrics.counterCredentialsRequest?.attributes || new Set; + counterConfigAttrs.forEach(counterConfigAttr => { + if (!validAttrs.has(counterConfigAttr)) { + throw new FgaValidationError(`Configuration.telemtry.metrics.counterCredentialsRequest attribute '${counterConfigAttrs}' is not a valid attribute`); + } + }); + + const histogramRequestDurationConfigAttrs = this.metrics.histogramRequestDuration?.attributes || new Set; + histogramRequestDurationConfigAttrs.forEach(histogramRequestDurationAttr => { + if (!validAttrs.has(histogramRequestDurationAttr)) { + throw new FgaValidationError(`Configuration.telemtry.metrics.histogramRequestDuration attribute '${histogramRequestDurationAttr}' is not a valid attribute`); + } + }); + + const histogramQueryDurationConfigAttrs = this.metrics.histogramQueryDuration?.attributes || new Set; + histogramQueryDurationConfigAttrs.forEach(histogramQueryDurationConfigAttr => { + if (!validAttrs.has(histogramQueryDurationConfigAttr)) { + throw new FgaValidationError(`Configuration.telemtry.metrics.histogramQueryDuration attribute '${histogramQueryDurationConfigAttrs}' is not a valid attribute`); + } + }); } } From c7b0191dc048aef82c5fca099af3249799e7a153 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Wed, 18 Sep 2024 22:32:45 -0500 Subject: [PATCH 17/30] make telemetry attributes methods static --- common.ts | 10 +++++----- credentials/credentials.ts | 9 ++++----- telemetry/attributes.ts | 6 +++--- tests/telemetry/attributes.test.ts | 21 ++++++++------------- tests/telemetry/metrics.test.ts | 2 +- 5 files changed, 21 insertions(+), 27 deletions(-) diff --git a/common.ts b/common.ts index 44432e3..23051ba 100644 --- a/common.ts +++ b/common.ts @@ -209,18 +209,18 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst const result: CallResult = { ...data }; setNotEnumerableProperty(result, "$response", response); - const telemetryAttributes = new TelemetryAttributes(); + // TODO shouldn't need a new instance per each request? const telemetryMetrics = new TelemetryMetrics(); let attributes: StringIndexable = {}; - attributes = telemetryAttributes.fromRequest({ + attributes = TelemetryAttributes.fromRequest({ start: start, credentials: credentials, attributes: methodAttributes, }); - attributes = telemetryAttributes.fromResponse({ + attributes = TelemetryAttributes.fromResponse({ response, credentials, attributes, @@ -230,7 +230,7 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst telemetryMetrics.histogram( TelemetryHistograms.queryDuration, parseInt(attributes[TelemetryAttribute.HttpServerRequestDuration] as string, 10), - telemetryAttributes.prepare( + TelemetryAttributes.prepare( attributes, configuration.telemetry.metrics.histogramQueryDuration.attributes ) @@ -241,7 +241,7 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst telemetryMetrics.histogram( TelemetryHistograms.requestDuration, Date.now() - start, - telemetryAttributes.prepare( + TelemetryAttributes.prepare( attributes, configuration.telemetry.metrics.histogramRequestDuration.attributes ) diff --git a/credentials/credentials.ts b/credentials/credentials.ts index 97d982f..ad3498a 100644 --- a/credentials/credentials.ts +++ b/credentials/credentials.ts @@ -161,26 +161,25 @@ export class Credentials { this.accessTokenExpiryDate = new Date(Date.now() + response.data.expires_in * 1000); } - // TODO is this the right check? if (this.telemetryConfig?.metrics?.counterCredentialsRequest?.attributes) { - const telemetryAttributes = new TelemetryAttributes(); + // TODO shouldn't need a new instance per each request? const telemetryMetrics = new TelemetryMetrics(); let attributes = {}; - attributes = telemetryAttributes.fromRequest({ + attributes = TelemetryAttributes.fromRequest({ credentials: clientCredentials, // resendCount: 0, // TODO: implement resend count tracking, not available in the current context attributes, }); - attributes = telemetryAttributes.fromResponse({ + attributes = TelemetryAttributes.fromResponse({ response, credentials: clientCredentials, attributes, }); - attributes = telemetryAttributes.prepare(attributes); + attributes = TelemetryAttributes.prepare(attributes); telemetryMetrics.counter(TelemetryCounters.credentialsRequest, 1, attributes); } diff --git a/telemetry/attributes.ts b/telemetry/attributes.ts index e4a2a03..4cbcedc 100644 --- a/telemetry/attributes.ts +++ b/telemetry/attributes.ts @@ -21,7 +21,7 @@ export enum TelemetryAttribute { } export class TelemetryAttributes { - prepare( + static prepare( attributes?: Record, filter?: Set ): Record { @@ -38,7 +38,7 @@ export class TelemetryAttributes { return result; } - fromRequest({ + static fromRequest({ userAgent, fgaMethod, httpMethod, @@ -77,7 +77,7 @@ export class TelemetryAttributes { return attributes; } - fromResponse({ + static fromResponse({ response, credentials, attributes = {}, diff --git a/tests/telemetry/attributes.test.ts b/tests/telemetry/attributes.test.ts index d5dad99..87dd934 100644 --- a/tests/telemetry/attributes.test.ts +++ b/tests/telemetry/attributes.test.ts @@ -1,11 +1,6 @@ import { TelemetryAttribute, TelemetryAttributes } from "../../telemetry/attributes"; describe("TelemetryAttributes", () => { - let telemetryAttributes: TelemetryAttributes; - - beforeEach(() => { - telemetryAttributes = new TelemetryAttributes(); - }); test("should prepare attributes correctly", () => { const attributes = { @@ -14,7 +9,7 @@ describe("TelemetryAttributes", () => { }; const filter = new Set([TelemetryAttribute.FgaClientRequestClientId]); - const prepared = telemetryAttributes.prepare(attributes, filter); + const prepared = TelemetryAttributes.prepare(attributes, filter); expect(prepared).toEqual({ "fga-client.request.client_id": "test-client-id" }); }); @@ -24,14 +19,14 @@ describe("TelemetryAttributes", () => { [TelemetryAttribute.HttpHost]: "example.com", [TelemetryAttribute.HttpResponseStatusCode]: 200, }; - expect(telemetryAttributes.prepare(attributes)).toEqual({}); + expect(TelemetryAttributes.prepare(attributes)).toEqual({}); }); test("should return an empty object when filter is provided but attributes is undefined", () => { const filter = new Set([ TelemetryAttribute.HttpHost, ]); - expect(telemetryAttributes.prepare(undefined, filter)).toEqual({}); + expect(TelemetryAttributes.prepare(undefined, filter)).toEqual({}); }); test("should return an empty object when none of the attributes are in the filter set", () => { @@ -42,11 +37,11 @@ describe("TelemetryAttributes", () => { const filter = new Set([ TelemetryAttribute.UserAgentOriginal, ]); - expect(telemetryAttributes.prepare(attributes, filter)).toEqual({}); + expect(TelemetryAttributes.prepare(attributes, filter)).toEqual({}); }); test("should create attributes from request correctly", () => { - const result = telemetryAttributes.fromRequest({ + const result = TelemetryAttributes.fromRequest({ userAgent: "Mozilla/5.0", fgaMethod: "GET", httpMethod: "POST", @@ -65,7 +60,7 @@ describe("TelemetryAttributes", () => { test("should create attributes from response correctly", () => { const response = { status: 200, headers: { "openfga-authorization-model-id": "model-id", "fga-query-duration-ms": "10" } }; - const result = telemetryAttributes.fromResponse({ response }); + const result = TelemetryAttributes.fromResponse({ response }); // Verify line 90 is covered - status is correctly set expect(result["http.response.status_code"]).toEqual(200); @@ -75,7 +70,7 @@ describe("TelemetryAttributes", () => { test("should handle response without status correctly", () => { const response = { headers: { "openfga-authorization-model-id": "model-id", "fga-query-duration-ms": "10" } }; - const result = telemetryAttributes.fromResponse({ response }); + const result = TelemetryAttributes.fromResponse({ response }); // Verify that no status code is set when response does not have a status expect(result["http.response.status_code"]).toBeUndefined(); @@ -86,7 +81,7 @@ describe("TelemetryAttributes", () => { test("should create attributes from response with client credentials", () => { const response = { status: 200, headers: {} }; const credentials = { method: "client_credentials", configuration: { clientId: "client-id" } }; - const result = telemetryAttributes.fromResponse({ response, credentials }); + const result = TelemetryAttributes.fromResponse({ response, credentials }); // Check that the client ID is set correctly from the credentials expect(result["http.response.status_code"]).toEqual(200); diff --git a/tests/telemetry/metrics.test.ts b/tests/telemetry/metrics.test.ts index a5adbc0..7a388c2 100644 --- a/tests/telemetry/metrics.test.ts +++ b/tests/telemetry/metrics.test.ts @@ -34,7 +34,7 @@ describe("TelemetryMetrics", () => { }); test("should handle creating metrics with custom attributes", () => { - const attributes = new TelemetryAttributes().prepare({ "http.host": "example.com" }); + const attributes = TelemetryAttributes.prepare({ "http.host": "example.com" }); const counter = telemetryMetrics.counter(TelemetryCounters.credentialsRequest, 3, attributes); expect(counter.add).toHaveBeenCalledWith(3, attributes); From fa28c482af7c07613899145cc03c0cfb3bcc420e Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Thu, 19 Sep 2024 12:44:59 -0500 Subject: [PATCH 18/30] use enums for metric values, simplify config --- configuration.ts | 4 +- telemetry/configuration.ts | 139 ++++++++++---------------- telemetry/metrics.ts | 14 ++- tests/index.test.ts | 4 - tests/telemetry/configuration.test.ts | 17 +++- 5 files changed, 80 insertions(+), 98 deletions(-) diff --git a/configuration.ts b/configuration.ts index 019a026..a9755d4 100644 --- a/configuration.ts +++ b/configuration.ts @@ -14,9 +14,7 @@ import { ApiTokenConfig, AuthCredentialsConfig, ClientCredentialsConfig, CredentialsMethod } from "./credentials/types"; import { FgaValidationError, } from "./errors"; import { assertParamExists, isWellFormedUlidString, isWellFormedUriString } from "./validation"; -import { TelemetryConfig, TelemetryConfiguration, validAttributes } from "./telemetry/configuration"; -import { TelemetryAttribute } from "./telemetry/attributes"; -import { count } from "console"; +import { TelemetryConfig, TelemetryConfiguration } from "./telemetry/configuration"; // default maximum number of retry const DEFAULT_MAX_RETRY = 15; diff --git a/telemetry/configuration.ts b/telemetry/configuration.ts index 8282222..7c894fa 100644 --- a/telemetry/configuration.ts +++ b/telemetry/configuration.ts @@ -1,80 +1,72 @@ import { FgaValidationError } from "../errors"; import { TelemetryAttribute } from "./attributes"; - - -const defaultAttributes = new Set([ - TelemetryAttribute.HttpHost, - TelemetryAttribute.HttpResponseStatusCode, - TelemetryAttribute.UserAgentOriginal, - TelemetryAttribute.FgaClientRequestMethod, - TelemetryAttribute.FgaClientRequestClientId, - TelemetryAttribute.FgaClientRequestStoreId, - TelemetryAttribute.FgaClientRequestModelId, - TelemetryAttribute.HttpRequestResendCount, - TelemetryAttribute.FgaClientResponseModelId, - - // These metrics are not included by default because they are usually less useful - // TelemetryAttribute.UrlScheme, - // TelemetryAttribute.HttpRequestMethod, - // TelemetryAttribute.UrlFull, - // TelemetryAttribute.HttpClientRequestDuration, - // TelemetryAttribute.HttpServerRequestDuration - - // This not included by default as it has a very high cardinality which could increase costs for users - // TelemetryAttribute.FgaClientUser -]); - -export class TelemetryMetricConfiguration implements TelemetryMetricConfig { - constructor( - public attributes: Set = defaultAttributes - ) {} -} - -// Drop this whole class. Do this logic in the calling class (TelemetryCOnfiguration) -export class TelemetryMetricsConfiguration implements MetricsConfig { - // instead of taking 3 individual metrics, accept a map. e.g., - // ["counterCredentialsRequest": TelemetryMetricConfig] - // only constructor will change - constructor( - public counterCredentialsRequest: TelemetryMetricConfig = new TelemetryMetricConfiguration(), - public histogramRequestDuration: TelemetryMetricConfig = new TelemetryMetricConfiguration(), - public histogramQueryDuration: TelemetryMetricConfig = new TelemetryMetricConfiguration() - ) { - if (!(counterCredentialsRequest instanceof TelemetryMetricConfiguration)) { - counterCredentialsRequest = new TelemetryMetricConfiguration(counterCredentialsRequest.attributes); - } - if (!(histogramRequestDuration instanceof TelemetryMetricConfiguration)) { - histogramRequestDuration = new TelemetryMetricConfiguration(histogramRequestDuration.attributes); - } - if (!(histogramQueryDuration instanceof TelemetryMetricConfiguration)) { - histogramQueryDuration = new TelemetryMetricConfiguration(histogramQueryDuration.attributes); - } - } -} +import { TelemetryMetric } from "./metrics"; export interface TelemetryMetricConfig { attributes: Set; } -export interface MetricsConfig { - counterCredentialsRequest: TelemetryMetricConfiguration; - histogramRequestDuration: TelemetryMetricConfiguration; - histogramQueryDuration: TelemetryMetricConfiguration; +export interface TelemetryConfig { + metrics: Record; } -export interface TelemetryConfig { - metrics: MetricsConfig +export class TelemetryMetricConfiguration implements TelemetryMetricConfig { + constructor( + public attributes: Set = TelemetryConfiguration.defaultAttributes + ) {} } export class TelemetryConfiguration implements TelemetryConfig { - constructor(public metrics: MetricsConfig = new TelemetryMetricsConfiguration()) { - if (!(metrics instanceof TelemetryMetricsConfiguration)) { - metrics = new TelemetryMetricsConfiguration(metrics.counterCredentialsRequest, metrics.histogramRequestDuration, metrics.histogramQueryDuration); - } - } + public static readonly defaultAttributes = new Set([ + TelemetryAttribute.HttpHost, + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UserAgentOriginal, + TelemetryAttribute.FgaClientRequestMethod, + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.FgaClientRequestStoreId, + TelemetryAttribute.FgaClientRequestModelId, + TelemetryAttribute.HttpRequestResendCount, + TelemetryAttribute.FgaClientResponseModelId, + + // These metrics are not included by default because they are usually less useful + // TelemetryAttribute.UrlScheme, + // TelemetryAttribute.HttpRequestMethod, + // TelemetryAttribute.UrlFull, + // TelemetryAttribute.HttpClientRequestDuration, + // TelemetryAttribute.HttpServerRequestDuration + + // This not included by default as it has a very high cardinality which could increase costs for users + // TelemetryAttribute.FgaClientUser + ]); + + public static readonly validAttriburtes = new Set([ + TelemetryAttribute.HttpHost, + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UserAgentOriginal, + TelemetryAttribute.FgaClientRequestMethod, + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.FgaClientRequestStoreId, + TelemetryAttribute.FgaClientRequestModelId, + TelemetryAttribute.HttpRequestResendCount, + TelemetryAttribute.FgaClientResponseModelId, + TelemetryAttribute.UrlScheme, + TelemetryAttribute.HttpRequestMethod, + TelemetryAttribute.UrlFull, + TelemetryAttribute.HttpClientRequestDuration, + TelemetryAttribute.HttpServerRequestDuration, + TelemetryAttribute.FgaClientUser + ]); + + constructor( + public metrics: Record = { + [TelemetryMetric.CounterCredentialsRequest]: new TelemetryMetricConfiguration(), + [TelemetryMetric.HistogramRequestDuration]: new TelemetryMetricConfiguration(), + [TelemetryMetric.HistogramQueryDuration]: new TelemetryMetricConfiguration(), + }, + ) {} public ensureValid(): void { - const validAttrs = validAttributes(); + const validAttrs = TelemetryConfiguration.validAttriburtes; const counterConfigAttrs = this.metrics.counterCredentialsRequest?.attributes || new Set; counterConfigAttrs.forEach(counterConfigAttr => { @@ -96,26 +88,5 @@ export class TelemetryConfiguration implements TelemetryConfig { throw new FgaValidationError(`Configuration.telemtry.metrics.histogramQueryDuration attribute '${histogramQueryDurationConfigAttrs}' is not a valid attribute`); } }); - } } - -export function validAttributes(): Set { - return new Set([ - TelemetryAttribute.HttpHost, - TelemetryAttribute.HttpResponseStatusCode, - TelemetryAttribute.UserAgentOriginal, - TelemetryAttribute.FgaClientRequestMethod, - TelemetryAttribute.FgaClientRequestClientId, - TelemetryAttribute.FgaClientRequestStoreId, - TelemetryAttribute.FgaClientRequestModelId, - TelemetryAttribute.HttpRequestResendCount, - TelemetryAttribute.FgaClientResponseModelId, - TelemetryAttribute.UrlScheme, - TelemetryAttribute.HttpRequestMethod, - TelemetryAttribute.UrlFull, - TelemetryAttribute.HttpClientRequestDuration, - TelemetryAttribute.HttpServerRequestDuration, - TelemetryAttribute.FgaClientUser - ]); -} diff --git a/telemetry/metrics.ts b/telemetry/metrics.ts index 55cf758..cce0280 100644 --- a/telemetry/metrics.ts +++ b/telemetry/metrics.ts @@ -1,10 +1,14 @@ -import { Counter, Histogram, Meter, ValueType } from "@opentelemetry/api"; -import { TelemetryConfiguration, TelemetryMetricsConfiguration } from "./configuration"; -import { TelemetryCounters, TelemetryCounter } from "./counters"; -import { TelemetryHistograms, TelemetryHistogram } from "./histograms"; -import { TelemetryAttributes, TelemetryAttribute } from "./attributes"; +import { Counter, Histogram, Meter } from "@opentelemetry/api"; +import { TelemetryCounter } from "./counters"; +import { TelemetryHistogram } from "./histograms"; import { metrics } from "@opentelemetry/api"; +export enum TelemetryMetric { + CounterCredentialsRequest = "counterCredentialsRequest", + HistogramRequestDuration = "histogramRequestDuration", + HistogramQueryDuration = "histogramQueryDuration", +} + export class TelemetryMetrics { private _meter: Meter | null = null; private _counters: Record = {}; diff --git a/tests/index.test.ts b/tests/index.test.ts index a682a3d..b3edb2c 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -23,12 +23,8 @@ import { FgaApiNotFoundError, FgaApiRateLimitExceededError, FgaApiValidationError, - FgaValidationError, OpenFgaApi, TelemetryAttribute, - TelemetryConfiguration, - TelemetryMetricConfiguration, - TelemetryMetricsConfiguration, } from "../index"; import { CallResult } from "../common"; import { GetDefaultRetryParams } from "../configuration"; diff --git a/tests/telemetry/configuration.test.ts b/tests/telemetry/configuration.test.ts index c2058c6..e4fb6f6 100644 --- a/tests/telemetry/configuration.test.ts +++ b/tests/telemetry/configuration.test.ts @@ -1,5 +1,6 @@ -import { TelemetryMetricConfiguration, TelemetryConfiguration, TelemetryMetricsConfiguration } from "../../telemetry/configuration"; +import { TelemetryMetricConfiguration, TelemetryConfiguration, TelemetryMetricConfig } from "../../telemetry/configuration"; import { TelemetryAttribute } from "../../telemetry/attributes"; +import { TelemetryMetric } from "../../dist"; describe("TelemetryMetricConfiguration", () => { test("should create a default TelemetryMetricConfiguration instance", () => { @@ -47,6 +48,18 @@ describe("TelemetryConfiguration", () => { test("should create a default TelemetryConfiguration instance", () => { const config = new TelemetryConfiguration(); - expect(config.metrics).toBeInstanceOf(TelemetryMetricsConfiguration); + const counters = config.metrics.counterCredentialsRequest; + expect(counters).toBeInstanceOf(TelemetryMetricConfiguration); + expect(counters.attributes).toEqual(TelemetryConfiguration.defaultAttributes); + + const histogramQueryDuration = config.metrics.histogramQueryDuration; + expect(histogramQueryDuration).toBeInstanceOf(TelemetryMetricConfiguration); + expect(histogramQueryDuration.attributes).toEqual(TelemetryConfiguration.defaultAttributes); + + const histogramRequestDuration = config.metrics.histogramRequestDuration; + expect(counters).toBeInstanceOf(TelemetryMetricConfiguration); + expect(counters.attributes).toEqual(TelemetryConfiguration.defaultAttributes); }); + + // TODO verify behavior for only specifying some of the metrics, what should the others be set to? }); From 997b0ae2231926c5887e622d73887945ae49ffc0 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Thu, 19 Sep 2024 13:25:57 -0500 Subject: [PATCH 19/30] add jsdocs to TelemetryConfig --- telemetry/configuration.ts | 72 ++++++++++++++++++++++++++++++++------ 1 file changed, 61 insertions(+), 11 deletions(-) diff --git a/telemetry/configuration.ts b/telemetry/configuration.ts index 7c894fa..fde339c 100644 --- a/telemetry/configuration.ts +++ b/telemetry/configuration.ts @@ -2,22 +2,55 @@ import { FgaValidationError } from "../errors"; import { TelemetryAttribute } from "./attributes"; import { TelemetryMetric } from "./metrics"; +/** + * Configuration for a telemetry metric. + * + * @interface TelemetryMetricConfig + * @property {Set} attributes - A set of attributes associated with the telemetry metric. + */ export interface TelemetryMetricConfig { attributes: Set; } +/** + * Represents the overall configuration for telemetry, including various metrics. + * + * @interface TelemetryConfig + * @property {Record} metrics - A record mapping telemetry metrics to their configurations. + */ export interface TelemetryConfig { metrics: Record; } +/** + * Provides the configuration for a specific telemetry metric. + * + * @class TelemetryMetricConfiguration + * @implements {TelemetryMetricConfig} + * @param {Set} [attributes=TelemetryConfiguration.defaultAttributes] - A set of attributes for the metric. Defaults to the standard attributes. + */ export class TelemetryMetricConfiguration implements TelemetryMetricConfig { constructor( public attributes: Set = TelemetryConfiguration.defaultAttributes ) {} } +/** + * Manages the overall telemetry configuration, including default and valid attributes. + * + * @class TelemetryConfiguration + * @implements {TelemetryConfig} + */ export class TelemetryConfiguration implements TelemetryConfig { - public static readonly defaultAttributes = new Set([ + + /** + * Default attributes for telemetry metrics. + * + * @static + * @readonly + * @type {Set} + */ + public static readonly defaultAttributes: Set = new Set([ TelemetryAttribute.HttpHost, TelemetryAttribute.HttpResponseStatusCode, TelemetryAttribute.UserAgentOriginal, @@ -27,7 +60,7 @@ export class TelemetryConfiguration implements TelemetryConfig { TelemetryAttribute.FgaClientRequestModelId, TelemetryAttribute.HttpRequestResendCount, TelemetryAttribute.FgaClientResponseModelId, - + // These metrics are not included by default because they are usually less useful // TelemetryAttribute.UrlScheme, // TelemetryAttribute.HttpRequestMethod, @@ -38,8 +71,15 @@ export class TelemetryConfiguration implements TelemetryConfig { // This not included by default as it has a very high cardinality which could increase costs for users // TelemetryAttribute.FgaClientUser ]); - - public static readonly validAttriburtes = new Set([ + + /** + * Valid attributes that can be used in telemetry metrics. + * + * @static + * @readonly + * @type {Set} + */ + public static readonly validAttriburtes: Set = new Set([ TelemetryAttribute.HttpHost, TelemetryAttribute.HttpResponseStatusCode, TelemetryAttribute.UserAgentOriginal, @@ -54,9 +94,14 @@ export class TelemetryConfiguration implements TelemetryConfig { TelemetryAttribute.UrlFull, TelemetryAttribute.HttpClientRequestDuration, TelemetryAttribute.HttpServerRequestDuration, - TelemetryAttribute.FgaClientUser + TelemetryAttribute.FgaClientUser, ]); + /** + * Creates an instance of TelemetryConfiguration. + * + * @param {Record} [metrics] - A record mapping telemetry metrics to their configurations. + */ constructor( public metrics: Record = { [TelemetryMetric.CounterCredentialsRequest]: new TelemetryMetricConfiguration(), @@ -65,27 +110,32 @@ export class TelemetryConfiguration implements TelemetryConfig { }, ) {} + /** + * Validates that the configured metrics use only valid attributes. + * + * @throws {FgaValidationError} Throws an error if any attribute in the metric configurations is invalid. + */ public ensureValid(): void { const validAttrs = TelemetryConfiguration.validAttriburtes; - const counterConfigAttrs = this.metrics.counterCredentialsRequest?.attributes || new Set; + const counterConfigAttrs = this.metrics.counterCredentialsRequest?.attributes || new Set(); counterConfigAttrs.forEach(counterConfigAttr => { if (!validAttrs.has(counterConfigAttr)) { - throw new FgaValidationError(`Configuration.telemtry.metrics.counterCredentialsRequest attribute '${counterConfigAttrs}' is not a valid attribute`); + throw new FgaValidationError(`Configuration.telemetry.metrics.counterCredentialsRequest attribute '${counterConfigAttr}' is not a valid attribute`); } }); - const histogramRequestDurationConfigAttrs = this.metrics.histogramRequestDuration?.attributes || new Set; + const histogramRequestDurationConfigAttrs = this.metrics.histogramRequestDuration?.attributes || new Set(); histogramRequestDurationConfigAttrs.forEach(histogramRequestDurationAttr => { if (!validAttrs.has(histogramRequestDurationAttr)) { - throw new FgaValidationError(`Configuration.telemtry.metrics.histogramRequestDuration attribute '${histogramRequestDurationAttr}' is not a valid attribute`); + throw new FgaValidationError(`Configuration.telemetry.metrics.histogramRequestDuration attribute '${histogramRequestDurationAttr}' is not a valid attribute`); } }); - const histogramQueryDurationConfigAttrs = this.metrics.histogramQueryDuration?.attributes || new Set; + const histogramQueryDurationConfigAttrs = this.metrics.histogramQueryDuration?.attributes || new Set(); histogramQueryDurationConfigAttrs.forEach(histogramQueryDurationConfigAttr => { if (!validAttrs.has(histogramQueryDurationConfigAttr)) { - throw new FgaValidationError(`Configuration.telemtry.metrics.histogramQueryDuration attribute '${histogramQueryDurationConfigAttrs}' is not a valid attribute`); + throw new FgaValidationError(`Configuration.telemetry.metrics.histogramQueryDuration attribute '${histogramQueryDurationConfigAttr}' is not a valid attribute`); } }); } From 26f53235f24cb8d197c43125f468e25388a56743 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Thu, 19 Sep 2024 13:31:53 -0500 Subject: [PATCH 20/30] fix bad auto import in test --- tests/telemetry/configuration.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/telemetry/configuration.test.ts b/tests/telemetry/configuration.test.ts index e4fb6f6..63b0aa6 100644 --- a/tests/telemetry/configuration.test.ts +++ b/tests/telemetry/configuration.test.ts @@ -1,6 +1,5 @@ import { TelemetryMetricConfiguration, TelemetryConfiguration, TelemetryMetricConfig } from "../../telemetry/configuration"; import { TelemetryAttribute } from "../../telemetry/attributes"; -import { TelemetryMetric } from "../../dist"; describe("TelemetryMetricConfiguration", () => { test("should create a default TelemetryMetricConfiguration instance", () => { From 004cd0acd1f20ed35e22d27056e7536b350484b7 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Thu, 19 Sep 2024 13:34:47 -0500 Subject: [PATCH 21/30] update example to show all histogram metrics by default --- example/opentelemetry/opentelemetry.mjs | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/example/opentelemetry/opentelemetry.mjs b/example/opentelemetry/opentelemetry.mjs index 745c357..a9f221c 100644 --- a/example/opentelemetry/opentelemetry.mjs +++ b/example/opentelemetry/opentelemetry.mjs @@ -1,6 +1,5 @@ import "dotenv/config"; -import { CredentialsMethod, FgaApiValidationError, OpenFgaClient, TelemetryAttribute, TelemetryMetricConfiguration, TelemetryMetricsConfiguration } from "@openfga/sdk"; -// import { TelemetryConfiguration } from "../../telemetry/configuration"; +import { CredentialsMethod, FgaApiValidationError, OpenFgaClient, TelemetryAttribute, TelemetryConfiguration } from "@openfga/sdk"; let credentials; if (process.env.FGA_CLIENT_ID) { @@ -23,25 +22,8 @@ const counterCredentialsRequestAttributes = new Set([ TelemetryAttribute.FgaClientRequestClientId ]); -const histogramRequestDurationAttributes = new Set([ - TelemetryAttribute.HttpResponseStatusCode, - TelemetryAttribute.UserAgentOriginal, - TelemetryAttribute.HttpRequestMethod, - TelemetryAttribute.FgaClientRequestClientId, - TelemetryAttribute.FgaClientRequestStoreId, - TelemetryAttribute.FgaClientResponseModelId, - TelemetryAttribute.HttpRequestResendCount, -]); - -const histogramQueryDurationAttributes = new Set([ - TelemetryAttribute.HttpResponseStatusCode, - TelemetryAttribute.UserAgentOriginal, - TelemetryAttribute.HttpRequestMethod, - TelemetryAttribute.FgaClientRequestClientId, - TelemetryAttribute.FgaClientRequestStoreId, - TelemetryAttribute.FgaClientResponseModelId, - TelemetryAttribute.HttpRequestResendCount, -]); +const histogramRequestDurationAttributes = TelemetryConfiguration.validAttriburtes; +const histogramQueryDurationAttributes = TelemetryConfiguration.validAttriburtes; const telemetryConfig = { metrics: { From 748995c42fb15b7c4d4b8329963232f94e7bc939 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Sun, 22 Sep 2024 21:58:04 -0500 Subject: [PATCH 22/30] attach metrics recorder to configuration so only one instance --- common.ts | 9 ++---- credentials/credentials.ts | 6 ++-- telemetry/configuration.ts | 21 +++++++++---- telemetry/metrics.ts | 3 +- tests/index.test.ts | 45 +++++++++++++-------------- tests/telemetry/configuration.test.ts | 31 ++++++++++++++++-- tests/telemetry/metrics.test.ts | 6 ++-- 7 files changed, 74 insertions(+), 47 deletions(-) diff --git a/common.ts b/common.ts index 23051ba..1c23123 100644 --- a/common.ts +++ b/common.ts @@ -26,7 +26,7 @@ import { } from "./errors"; import { setNotEnumerableProperty } from "./utils"; import { TelemetryAttribute, TelemetryAttributes } from "./telemetry/attributes"; -import { TelemetryMetrics } from "./telemetry/metrics"; +import { MetricRecorder } from "./telemetry/metrics"; import { TelemetryHistograms } from "./telemetry/histograms"; /** @@ -209,9 +209,6 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst const result: CallResult = { ...data }; setNotEnumerableProperty(result, "$response", response); - // TODO shouldn't need a new instance per each request? - const telemetryMetrics = new TelemetryMetrics(); - let attributes: StringIndexable = {}; attributes = TelemetryAttributes.fromRequest({ @@ -227,7 +224,7 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst }); if (configuration.telemetry?.metrics?.histogramQueryDuration) { - telemetryMetrics.histogram( + configuration.telemetry.recorder.histogram( TelemetryHistograms.queryDuration, parseInt(attributes[TelemetryAttribute.HttpServerRequestDuration] as string, 10), TelemetryAttributes.prepare( @@ -238,7 +235,7 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst } if (configuration.telemetry?.metrics?.histogramRequestDuration) { - telemetryMetrics.histogram( + configuration.telemetry.recorder.histogram( TelemetryHistograms.requestDuration, Date.now() - start, TelemetryAttributes.prepare( diff --git a/credentials/credentials.ts b/credentials/credentials.ts index ad3498a..289a49a 100644 --- a/credentials/credentials.ts +++ b/credentials/credentials.ts @@ -18,7 +18,7 @@ import { FgaApiAuthenticationError, FgaApiError, FgaError, FgaValidationError } import { attemptHttpRequest } from "../common"; import { AuthCredentialsConfig, ClientCredentialsConfig, CredentialsMethod } from "./types"; import { TelemetryAttributes } from "../telemetry/attributes"; -import { TelemetryMetrics } from "../telemetry/metrics"; +import { MetricRecorder } from "../telemetry/metrics"; import { TelemetryCounters } from "../telemetry/counters"; import { TelemetryConfiguration } from "../telemetry/configuration"; @@ -162,8 +162,6 @@ export class Credentials { } if (this.telemetryConfig?.metrics?.counterCredentialsRequest?.attributes) { - // TODO shouldn't need a new instance per each request? - const telemetryMetrics = new TelemetryMetrics(); let attributes = {}; @@ -180,7 +178,7 @@ export class Credentials { }); attributes = TelemetryAttributes.prepare(attributes); - telemetryMetrics.counter(TelemetryCounters.credentialsRequest, 1, attributes); + this.telemetryConfig.recorder.counter(TelemetryCounters.credentialsRequest, 1, attributes); } return this.accessToken; diff --git a/telemetry/configuration.ts b/telemetry/configuration.ts index fde339c..bf8763d 100644 --- a/telemetry/configuration.ts +++ b/telemetry/configuration.ts @@ -1,6 +1,6 @@ import { FgaValidationError } from "../errors"; import { TelemetryAttribute } from "./attributes"; -import { TelemetryMetric } from "./metrics"; +import { TelemetryMetric, MetricRecorder } from "./metrics"; /** * Configuration for a telemetry metric. @@ -19,7 +19,7 @@ export interface TelemetryMetricConfig { * @property {Record} metrics - A record mapping telemetry metrics to their configurations. */ export interface TelemetryConfig { - metrics: Record; + metrics: Partial>; } /** @@ -100,15 +100,24 @@ export class TelemetryConfiguration implements TelemetryConfig { /** * Creates an instance of TelemetryConfiguration. * - * @param {Record} [metrics] - A record mapping telemetry metrics to their configurations. + * @param {Partial>} [metrics] - A record mapping telemetry metrics to their configurations. */ constructor( - public metrics: Record = { + public metrics: Partial> = {}, + public recorder: MetricRecorder = new MetricRecorder(), + ) { + const defaultMetrics: Record = { [TelemetryMetric.CounterCredentialsRequest]: new TelemetryMetricConfiguration(), [TelemetryMetric.HistogramRequestDuration]: new TelemetryMetricConfiguration(), [TelemetryMetric.HistogramQueryDuration]: new TelemetryMetricConfiguration(), - }, - ) {} + }; + + // Merge provided metrics with default metrics + this.metrics = { + ...defaultMetrics, + ...metrics, + }; + } /** * Validates that the configured metrics use only valid attributes. diff --git a/telemetry/metrics.ts b/telemetry/metrics.ts index cce0280..7e60c0f 100644 --- a/telemetry/metrics.ts +++ b/telemetry/metrics.ts @@ -9,7 +9,8 @@ export enum TelemetryMetric { HistogramQueryDuration = "histogramQueryDuration", } -export class TelemetryMetrics { +export class MetricRecorder { + private _meter: Meter | null = null; private _counters: Record = {}; private _histograms: Record = {}; diff --git a/tests/index.test.ts b/tests/index.test.ts index b3edb2c..06cbbcb 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -251,34 +251,11 @@ describe("OpenFGA SDK", function () { }); it("should only accept valid telemetry attributes", async () => { - // const metricConfig = new TelemetryMetricConfiguration(new Set); - // const metricsConfig = new TelemetryMetricsConfiguration( - // metricConfig, - // metricConfig, - // metricConfig, - // ); - // const telConfig = new TelemetryConfiguration(metricsConfig); - - // const telemetryConfig = { - // metrics: { - // counterCredentialsRequest: { - // attributes: new Set - // }, - // histogramRequestDuration: { - // attributes: new Set - // }, - // histogramQueryDuration: { - // attributes: new Set - // } - // } - // }; - + expect( () => new OpenFgaApi({ ...baseConfig, - // telemetry: telemetryConfig, - // telemetry: telConfig, telemetry: { metrics: { counterCredentialsRequest: { @@ -295,6 +272,26 @@ describe("OpenFGA SDK", function () { }) ).toThrow(); }); + + it("should only accept valid telemetry metrics", async () => { + + expect( + () => + new OpenFgaApi({ + ...baseConfig, + telemetry: { + metrics: { + histogramRequestDuration: { + attributes: new Set + }, + counterCredentialsRequest: { + attributes: ["JUNK"] as any + }, + } + } + }) + ).toThrow(); + }); }); describe("error handling", () => { diff --git a/tests/telemetry/configuration.test.ts b/tests/telemetry/configuration.test.ts index 63b0aa6..1723a16 100644 --- a/tests/telemetry/configuration.test.ts +++ b/tests/telemetry/configuration.test.ts @@ -1,5 +1,6 @@ import { TelemetryMetricConfiguration, TelemetryConfiguration, TelemetryMetricConfig } from "../../telemetry/configuration"; import { TelemetryAttribute } from "../../telemetry/attributes"; +import { TelemetryMetric } from "../../telemetry/metrics"; describe("TelemetryMetricConfiguration", () => { test("should create a default TelemetryMetricConfiguration instance", () => { @@ -49,15 +50,39 @@ describe("TelemetryConfiguration", () => { const counters = config.metrics.counterCredentialsRequest; expect(counters).toBeInstanceOf(TelemetryMetricConfiguration); - expect(counters.attributes).toEqual(TelemetryConfiguration.defaultAttributes); + expect(counters?.attributes).toEqual(TelemetryConfiguration.defaultAttributes); const histogramQueryDuration = config.metrics.histogramQueryDuration; expect(histogramQueryDuration).toBeInstanceOf(TelemetryMetricConfiguration); - expect(histogramQueryDuration.attributes).toEqual(TelemetryConfiguration.defaultAttributes); + expect(histogramQueryDuration?.attributes).toEqual(TelemetryConfiguration.defaultAttributes); const histogramRequestDuration = config.metrics.histogramRequestDuration; expect(counters).toBeInstanceOf(TelemetryMetricConfiguration); - expect(counters.attributes).toEqual(TelemetryConfiguration.defaultAttributes); + expect(counters?.attributes).toEqual(TelemetryConfiguration.defaultAttributes); + }); + + test("should use defaults if not all metrics defined", () => { + const config = new TelemetryConfiguration({ + [TelemetryMetric.CounterCredentialsRequest]: new TelemetryMetricConfiguration(), + [TelemetryMetric.HistogramQueryDuration]: new TelemetryMetricConfiguration( + new Set([ + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UrlScheme, + TelemetryAttribute.HttpRequestMethod, + ]) + ), + }); + + expect(config.metrics.counterCredentialsRequest?.attributes).toEqual(TelemetryConfiguration.defaultAttributes); + expect(config.metrics.histogramQueryDuration?.attributes).toEqual(new Set( + new Set([ + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UrlScheme, + TelemetryAttribute.HttpRequestMethod, + ]))); + expect(config.metrics.histogramRequestDuration?.attributes).toEqual(TelemetryConfiguration.defaultAttributes); }); // TODO verify behavior for only specifying some of the metrics, what should the others be set to? diff --git a/tests/telemetry/metrics.test.ts b/tests/telemetry/metrics.test.ts index 7a388c2..7414194 100644 --- a/tests/telemetry/metrics.test.ts +++ b/tests/telemetry/metrics.test.ts @@ -1,4 +1,4 @@ -import { TelemetryMetrics } from "../../telemetry/metrics"; +import { MetricRecorder } from "../../telemetry/metrics"; import { TelemetryCounters } from "../../telemetry/counters"; import { TelemetryHistograms } from "../../telemetry/histograms"; import { TelemetryAttributes } from "../../telemetry/attributes"; @@ -13,10 +13,10 @@ jest.mock("@opentelemetry/api", () => ({ })); describe("TelemetryMetrics", () => { - let telemetryMetrics: TelemetryMetrics; + let telemetryMetrics: MetricRecorder; beforeEach(() => { - telemetryMetrics = new TelemetryMetrics(); + telemetryMetrics = new MetricRecorder(); }); test("should create a counter and add a value", () => { From bfa339b637b1fb37cb525cfb40604e018329204f Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 23 Sep 2024 12:28:22 -0500 Subject: [PATCH 23/30] update telemetry docs --- docs/opentelemetry.md | 97 ++++++++++++++++++++++++++++++++++------- telemetry/attributes.ts | 5 +-- 2 files changed, 83 insertions(+), 19 deletions(-) diff --git a/docs/opentelemetry.md b/docs/opentelemetry.md index 24765a2..ee83c7c 100644 --- a/docs/opentelemetry.md +++ b/docs/opentelemetry.md @@ -10,25 +10,90 @@ In cases when metrics events are sent, they will not be viewable outside of infr ### Supported Metrics -| Metric Name | Type | Description | -|---------------------------------|-----------|---------------------------------------------------------------------------------| -| `fga-client.request.duration` | Histogram | The total request time for FGA requests | -| `fga-client.query.duration` | Histogram | The amount of time the FGA server took to process the request | -|` fga-client.credentials.request`| Counter | The total number of times a new token was requested when using ClientCredentials| +| Metric Name | Type | Enabled by default | Description | +|---------------------------------|-----------|--------------------|---------------------------------------------------------------------------------| +| `fga-client.request.duration` | Histogram | Yes | The total request time for FGA requests | +| `fga-client.query.duration` | Histogram | Yes | The amount of time the FGA server took to process the request | +|` fga-client.credentials.request`| Counter | Yes | The total number of times a new token was requested when using ClientCredentials| ### Supported attributes -| Attribute Name | Type | Description | -|--------------------------------|----------|-------------------------------------------------------------------------------------| -| `fga-client.response.model_id` | `string` | The authorization model ID that the FGA server used | -| `fga-client.request.method` | `string` | The FGA method/action that was performed | -| `fga-client.request.store_id` | `string` | The store ID that was sent as part of the request | -| `fga-client.request.model_id` | `string` | The authorization model ID that was sent as part of the request, if any | -| `fga-client.request.client_id` | `string` | The client ID associated with the request, if any | -| `fga-client.user` | `string` | The user that is associated with the action of the request for check and list users | -| `http.status_code ` | `int` | The status code of the response | -| `http.method` | `string` | The HTTP method for the request | -| `http.host` | `string` | Host identifier of the origin the request was sent to | +| Attribute Name | Type | Enabled by default | Description | +|--------------------------------|-----------|--------------------|-------------------------------------------------------------------------------------| +| `fga-client.response.model_id` | `string` | Yes | The authorization model ID that the FGA server used | +| `fga-client.request.method` | `string` | Yes | The FGA method/action that was performed | +| `fga-client.request.store_id` | `string` | Yes | The store ID that was sent as part of the request | +| `fga-client.request.model_id` | `string` | Yes | The authorization model ID that was sent as part of the request, if any | +| `fga-client.request.client_id` | `string` | Yes | The client ID associated with the request, if any | +| `fga-client.user` | `string` | No | The user that is associated with the action of the request for check and list users | +| `http.status_code ` | `int` | Yes | The status code of the response | +| `http.request.method` | `string` | No | The HTTP method for the request | +| `http.host` | `string` | Yes | Host identifier of the origin the request was sent to | +| `user_agent.original` | `string` | Yes | The user agent used in the query | +| `url.full` | `string` | No | The full URL of the request | +| `url.scheme` | `string` | No | HTTP Scheme of the request (http/https) | +| `http.request.resend_count` | `int` | Yes | The number of retries attempted | +| `http.client.request.duration` | `int` | No | Time taken by the FGA server to process and evaluate the request, in milliseconds | +| `http.server.request.duration` | `int` | No | The number of retries attempted | + +## Default attributes + +Not all attributes are enabled by default. + +Some attributes, like `fga-client.user` have been disabled by default due to their high cardinality, which may result for very high costs when using some SaaS metric collectors. If you expect to have a high cardinality for a specific attribute, you can disable it by updating the telemetry configuration accordingly. + +If your configuration does not specify a given metric, the default attributes for that metric will be used. + + +```javascript +// define desired telemetry options +const telemetryConfig = { + metrics: { + counterCredentialsRequest: { + attributes: new Set([ + TelemetryAttribute.UrlScheme, + TelemetryAttribute.UserAgentOriginal, + TelemetryAttribute.HttpRequestMethod, + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.FgaClientRequestStoreId, + TelemetryAttribute.FgaClientRequestModelId, + TelemetryAttribute.HttpRequestResendCount, + ]) + }, + histogramRequestDuration: { + attributes: new Set([ + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UserAgentOriginal, + TelemetryAttribute.FgaClientRequestMethod, + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.FgaClientRequestStoreId, + TelemetryAttribute.FgaClientRequestModelId, + TelemetryAttribute.HttpRequestResendCount, + ]) + }, + histogramQueryDuration: { + attributes: new Set([ + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UserAgentOriginal, + TelemetryAttribute.FgaClientRequestMethod, + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.FgaClientRequestStoreId, + TelemetryAttribute.FgaClientRequestModelId, + TelemetryAttribute.HttpRequestResendCount, + ]) + } + } +}; + +const fgaClient = new OpenFgaClient({ + apiUrl: process.env.FGA_API_URL, + storeId: process.env.FGA_STORE_ID, + authorizationModelId: process.env.FGA_MODEL_ID, + credentials, + telemetry: telemetryConfig, +}); + +``` ## Example diff --git a/telemetry/attributes.ts b/telemetry/attributes.ts index 4cbcedc..cba972e 100644 --- a/telemetry/attributes.ts +++ b/telemetry/attributes.ts @@ -7,13 +7,11 @@ export enum TelemetryAttribute { FgaClientRequestStoreId = "fga-client.request.store_id", FgaClientResponseModelId = "fga-client.response.model_id", FgaClientUser = "fga-client.user", - // remove this attribute, keep as metric HttpClientRequestDuration = "http.client.request.duration", HttpHost = "http.host", HttpRequestMethod = "http.request.method", HttpRequestResendCount = "http.request.resend_count", HttpResponseStatusCode = "http.response.status_code", - // remove this attribute, keep as metric HttpServerRequestDuration = "http.server.request.duration", UrlScheme = "url.scheme", UrlFull = "url.full", @@ -26,7 +24,8 @@ export class TelemetryAttributes { filter?: Set ): Record { attributes = attributes || {}; - filter = filter || new Set(); + // ensure we are always using a set + filter = new Set(filter) || new Set(); const result: Record = {}; for (const key in attributes) { From 5d104a67120e0cf799ac0020b27a28561a99c3cb Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 23 Sep 2024 14:28:31 -0500 Subject: [PATCH 24/30] remove redundant test --- tests/index.test.ts | 20 -------------------- tests/telemetry/configuration.test.ts | 2 -- 2 files changed, 22 deletions(-) diff --git a/tests/index.test.ts b/tests/index.test.ts index 06cbbcb..9867ab0 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -272,26 +272,6 @@ describe("OpenFGA SDK", function () { }) ).toThrow(); }); - - it("should only accept valid telemetry metrics", async () => { - - expect( - () => - new OpenFgaApi({ - ...baseConfig, - telemetry: { - metrics: { - histogramRequestDuration: { - attributes: new Set - }, - counterCredentialsRequest: { - attributes: ["JUNK"] as any - }, - } - } - }) - ).toThrow(); - }); }); describe("error handling", () => { diff --git a/tests/telemetry/configuration.test.ts b/tests/telemetry/configuration.test.ts index 1723a16..cab9be9 100644 --- a/tests/telemetry/configuration.test.ts +++ b/tests/telemetry/configuration.test.ts @@ -84,6 +84,4 @@ describe("TelemetryConfiguration", () => { ]))); expect(config.metrics.histogramRequestDuration?.attributes).toEqual(TelemetryConfiguration.defaultAttributes); }); - - // TODO verify behavior for only specifying some of the metrics, what should the others be set to? }); From e5e663522635fcc235b434e456ce6212d872e4b4 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 23 Sep 2024 18:59:15 -0500 Subject: [PATCH 25/30] updates from code review --- telemetry/configuration.ts | 52 +++++++--------- telemetry/metrics.ts | 10 --- tests/telemetry/configuration.test.ts | 90 +++++++-------------------- 3 files changed, 45 insertions(+), 107 deletions(-) diff --git a/telemetry/configuration.ts b/telemetry/configuration.ts index bf8763d..9cb1767 100644 --- a/telemetry/configuration.ts +++ b/telemetry/configuration.ts @@ -9,7 +9,7 @@ import { TelemetryMetric, MetricRecorder } from "./metrics"; * @property {Set} attributes - A set of attributes associated with the telemetry metric. */ export interface TelemetryMetricConfig { - attributes: Set; + attributes?: Set; } /** @@ -19,20 +19,7 @@ export interface TelemetryMetricConfig { * @property {Record} metrics - A record mapping telemetry metrics to their configurations. */ export interface TelemetryConfig { - metrics: Partial>; -} - -/** - * Provides the configuration for a specific telemetry metric. - * - * @class TelemetryMetricConfiguration - * @implements {TelemetryMetricConfig} - * @param {Set} [attributes=TelemetryConfiguration.defaultAttributes] - A set of attributes for the metric. Defaults to the standard attributes. - */ -export class TelemetryMetricConfiguration implements TelemetryMetricConfig { - constructor( - public attributes: Set = TelemetryConfiguration.defaultAttributes - ) {} + metrics?: Partial>; } /** @@ -43,6 +30,8 @@ export class TelemetryMetricConfiguration implements TelemetryMetricConfig { */ export class TelemetryConfiguration implements TelemetryConfig { + public readonly recorder: MetricRecorder = new MetricRecorder(); + /** * Default attributes for telemetry metrics. * @@ -103,20 +92,21 @@ export class TelemetryConfiguration implements TelemetryConfig { * @param {Partial>} [metrics] - A record mapping telemetry metrics to their configurations. */ constructor( - public metrics: Partial> = {}, - public recorder: MetricRecorder = new MetricRecorder(), + public metrics?: Partial>, ) { - const defaultMetrics: Record = { - [TelemetryMetric.CounterCredentialsRequest]: new TelemetryMetricConfiguration(), - [TelemetryMetric.HistogramRequestDuration]: new TelemetryMetricConfiguration(), - [TelemetryMetric.HistogramQueryDuration]: new TelemetryMetricConfiguration(), - }; - - // Merge provided metrics with default metrics - this.metrics = { - ...defaultMetrics, - ...metrics, - }; + if (!metrics) { + this.metrics = { + [TelemetryMetric.CounterCredentialsRequest]: {attributes: TelemetryConfiguration.defaultAttributes}, + [TelemetryMetric.HistogramRequestDuration]: {attributes: TelemetryConfiguration.defaultAttributes}, + [TelemetryMetric.HistogramQueryDuration]: {attributes: TelemetryConfiguration.defaultAttributes}, + }; + } else { + this.metrics = { + [TelemetryMetric.CounterCredentialsRequest]: metrics[TelemetryMetric.CounterCredentialsRequest] || undefined, + [TelemetryMetric.HistogramRequestDuration]: metrics[TelemetryMetric.HistogramRequestDuration] || undefined, + [TelemetryMetric.HistogramQueryDuration]: metrics[TelemetryMetric.HistogramQueryDuration] || undefined, + }; + } } /** @@ -127,21 +117,21 @@ export class TelemetryConfiguration implements TelemetryConfig { public ensureValid(): void { const validAttrs = TelemetryConfiguration.validAttriburtes; - const counterConfigAttrs = this.metrics.counterCredentialsRequest?.attributes || new Set(); + const counterConfigAttrs = this.metrics?.counterCredentialsRequest?.attributes || new Set(); counterConfigAttrs.forEach(counterConfigAttr => { if (!validAttrs.has(counterConfigAttr)) { throw new FgaValidationError(`Configuration.telemetry.metrics.counterCredentialsRequest attribute '${counterConfigAttr}' is not a valid attribute`); } }); - const histogramRequestDurationConfigAttrs = this.metrics.histogramRequestDuration?.attributes || new Set(); + const histogramRequestDurationConfigAttrs = this.metrics?.histogramRequestDuration?.attributes || new Set(); histogramRequestDurationConfigAttrs.forEach(histogramRequestDurationAttr => { if (!validAttrs.has(histogramRequestDurationAttr)) { throw new FgaValidationError(`Configuration.telemetry.metrics.histogramRequestDuration attribute '${histogramRequestDurationAttr}' is not a valid attribute`); } }); - const histogramQueryDurationConfigAttrs = this.metrics.histogramQueryDuration?.attributes || new Set(); + const histogramQueryDurationConfigAttrs = this.metrics?.histogramQueryDuration?.attributes || new Set(); histogramQueryDurationConfigAttrs.forEach(histogramQueryDurationConfigAttr => { if (!validAttrs.has(histogramQueryDurationConfigAttr)) { throw new FgaValidationError(`Configuration.telemetry.metrics.histogramQueryDuration attribute '${histogramQueryDurationConfigAttr}' is not a valid attribute`); diff --git a/telemetry/metrics.ts b/telemetry/metrics.ts index 7e60c0f..4bf781b 100644 --- a/telemetry/metrics.ts +++ b/telemetry/metrics.ts @@ -15,16 +15,6 @@ export class MetricRecorder { private _counters: Record = {}; private _histograms: Record = {}; - constructor( - meter?: Meter, - counters?: Record, - histograms?: Record - ) { - this._meter = meter || null; - this._counters = counters || {}; - this._histograms = histograms || {}; - } - meter(): Meter { if (!this._meter) { this._meter = metrics.getMeter("@openfga/sdk", "0.6.3"); diff --git a/tests/telemetry/configuration.test.ts b/tests/telemetry/configuration.test.ts index cab9be9..ade0081 100644 --- a/tests/telemetry/configuration.test.ts +++ b/tests/telemetry/configuration.test.ts @@ -1,87 +1,45 @@ -import { TelemetryMetricConfiguration, TelemetryConfiguration, TelemetryMetricConfig } from "../../telemetry/configuration"; +import { TelemetryConfiguration, TelemetryMetricConfig } from "../../telemetry/configuration"; import { TelemetryAttribute } from "../../telemetry/attributes"; import { TelemetryMetric } from "../../telemetry/metrics"; -describe("TelemetryMetricConfiguration", () => { - test("should create a default TelemetryMetricConfiguration instance", () => { - const config = new TelemetryMetricConfiguration(); - - expect(config.attributes.has(TelemetryAttribute.HttpHost)); - expect(config.attributes.has(TelemetryAttribute.HttpResponseStatusCode)); - expect(config.attributes.has(TelemetryAttribute.UserAgentOriginal)); - expect(config.attributes.has(TelemetryAttribute.HttpRequestMethod)); - expect(config.attributes.has(TelemetryAttribute.FgaClientRequestMethod)); - expect(config.attributes.has(TelemetryAttribute.FgaClientRequestClientId)); - expect(config.attributes.has(TelemetryAttribute.FgaClientRequestStoreId)); - expect(config.attributes.has(TelemetryAttribute.FgaClientRequestModelId)); - expect(config.attributes.has(TelemetryAttribute.HttpRequestResendCount)); - expect(config.attributes.has(TelemetryAttribute.FgaClientResponseModelId)); - - // should not be there - expect(config.attributes.has(TelemetryAttribute.UrlScheme)).toBe(false); - expect(config.attributes.has(TelemetryAttribute.HttpRequestMethod)).toBe(false); - expect(config.attributes.has(TelemetryAttribute.UrlFull)).toBe(false); - expect(config.attributes.has(TelemetryAttribute.FgaClientUser)).toBe(false); - }); - - test("should return correct attributes based on enabled properties", () => { - const config = new TelemetryMetricConfiguration( - new Set([ - TelemetryAttribute.FgaClientRequestClientId, - TelemetryAttribute.HttpResponseStatusCode, - TelemetryAttribute.UrlScheme, - TelemetryAttribute.HttpRequestMethod, - ]) - ); - - const attributes = config.attributes; - - expect(attributes.size).toBe(4); - expect(attributes.has(TelemetryAttribute.FgaClientRequestClientId)); - expect(attributes.has(TelemetryAttribute.HttpResponseStatusCode)); - expect(attributes.has(TelemetryAttribute.UrlScheme)); - expect(attributes.has(TelemetryAttribute.HttpRequestMethod)); - }); -}); - describe("TelemetryConfiguration", () => { - test("should create a default TelemetryConfiguration instance", () => { - const config = new TelemetryConfiguration(); - - const counters = config.metrics.counterCredentialsRequest; - expect(counters).toBeInstanceOf(TelemetryMetricConfiguration); - expect(counters?.attributes).toEqual(TelemetryConfiguration.defaultAttributes); - - const histogramQueryDuration = config.metrics.histogramQueryDuration; - expect(histogramQueryDuration).toBeInstanceOf(TelemetryMetricConfiguration); - expect(histogramQueryDuration?.attributes).toEqual(TelemetryConfiguration.defaultAttributes); - - const histogramRequestDuration = config.metrics.histogramRequestDuration; - expect(counters).toBeInstanceOf(TelemetryMetricConfiguration); - expect(counters?.attributes).toEqual(TelemetryConfiguration.defaultAttributes); - }); - test("should use defaults if not all metrics defined", () => { const config = new TelemetryConfiguration({ - [TelemetryMetric.CounterCredentialsRequest]: new TelemetryMetricConfiguration(), - [TelemetryMetric.HistogramQueryDuration]: new TelemetryMetricConfiguration( - new Set([ + [TelemetryMetric.CounterCredentialsRequest]: {}, + [TelemetryMetric.HistogramQueryDuration]: { + attributes: new Set([ TelemetryAttribute.FgaClientRequestClientId, TelemetryAttribute.HttpResponseStatusCode, TelemetryAttribute.UrlScheme, TelemetryAttribute.HttpRequestMethod, ]) - ), + } }); - expect(config.metrics.counterCredentialsRequest?.attributes).toEqual(TelemetryConfiguration.defaultAttributes); - expect(config.metrics.histogramQueryDuration?.attributes).toEqual(new Set( + expect(config.metrics?.counterCredentialsRequest?.attributes).toEqual(undefined); + expect(config.metrics?.histogramQueryDuration?.attributes).toEqual(new Set( new Set([ TelemetryAttribute.FgaClientRequestClientId, TelemetryAttribute.HttpResponseStatusCode, TelemetryAttribute.UrlScheme, TelemetryAttribute.HttpRequestMethod, ]))); - expect(config.metrics.histogramRequestDuration?.attributes).toEqual(TelemetryConfiguration.defaultAttributes); + expect(config.metrics?.histogramRequestDuration?.attributes).toEqual(undefined); }); + + test("should use defaults", () => { + const config = new TelemetryConfiguration(); + + expect(config.metrics?.counterCredentialsRequest?.attributes).toEqual(TelemetryConfiguration.defaultAttributes); + expect(config.metrics?.histogramQueryDuration?.attributes).toEqual(TelemetryConfiguration.defaultAttributes); + expect(config.metrics?.histogramRequestDuration?.attributes).toEqual(TelemetryConfiguration.defaultAttributes); + }); + + test("should be undefined if empty object passed", () => { + const config = new TelemetryConfiguration({}); + + expect(config.metrics?.counterCredentialsRequest?.attributes).toEqual(undefined); + expect(config.metrics?.histogramQueryDuration?.attributes).toEqual(undefined); + expect(config.metrics?.histogramRequestDuration?.attributes).toEqual(undefined); + }); }); From bbb7976a3c47cce583e2426ba533b055108111ef Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Mon, 23 Sep 2024 18:59:36 -0500 Subject: [PATCH 26/30] update example --- example/opentelemetry/opentelemetry.mjs | 49 ++++++++++++++++--------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/example/opentelemetry/opentelemetry.mjs b/example/opentelemetry/opentelemetry.mjs index a9f221c..87635de 100644 --- a/example/opentelemetry/opentelemetry.mjs +++ b/example/opentelemetry/opentelemetry.mjs @@ -1,5 +1,5 @@ import "dotenv/config"; -import { CredentialsMethod, FgaApiValidationError, OpenFgaClient, TelemetryAttribute, TelemetryConfiguration } from "@openfga/sdk"; +import { CredentialsMethod, FgaApiValidationError, OpenFgaClient, TelemetryAttribute, TelemetryConfiguration, TelemetryMetric } from "@openfga/sdk"; let credentials; if (process.env.FGA_CLIENT_ID) { @@ -14,27 +14,40 @@ if (process.env.FGA_CLIENT_ID) { }; } -// define desired telemetry options -const counterCredentialsRequestAttributes = new Set([ - TelemetryAttribute.HttpHost, - TelemetryAttribute.HttpResponseStatusCode, - TelemetryAttribute.UserAgentOriginal, - TelemetryAttribute.FgaClientRequestClientId -]); - -const histogramRequestDurationAttributes = TelemetryConfiguration.validAttriburtes; -const histogramQueryDurationAttributes = TelemetryConfiguration.validAttriburtes; - const telemetryConfig = { metrics: { - counterCredentialsRequest: { - attributes: counterCredentialsRequestAttributes + [TelemetryMetric.CounterCredentialsRequest]: { + attributes: new Set([ + TelemetryAttribute.UrlScheme, + TelemetryAttribute.UserAgentOriginal, + TelemetryAttribute.HttpRequestMethod, + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.FgaClientRequestStoreId, + TelemetryAttribute.FgaClientRequestModelId, + TelemetryAttribute.HttpRequestResendCount, + ]) }, - histogramRequestDuration: { - attributes: histogramRequestDurationAttributes + [TelemetryMetric.HistogramRequestDuration]: { + attributes: new Set([ + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UserAgentOriginal, + TelemetryAttribute.FgaClientRequestMethod, + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.FgaClientRequestStoreId, + TelemetryAttribute.FgaClientRequestModelId, + TelemetryAttribute.HttpRequestResendCount, + ]) }, - histogramQueryDuration: { - attributes: histogramQueryDurationAttributes + [TelemetryMetric.HistogramQueryDuration]: { + attributes: new Set([ + TelemetryAttribute.HttpResponseStatusCode, + TelemetryAttribute.UserAgentOriginal, + TelemetryAttribute.FgaClientRequestMethod, + TelemetryAttribute.FgaClientRequestClientId, + TelemetryAttribute.FgaClientRequestStoreId, + TelemetryAttribute.FgaClientRequestModelId, + TelemetryAttribute.HttpRequestResendCount, + ]) } } }; From 33c98b86273b8219f80685bbc45db1d4420eb72b Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Thu, 26 Sep 2024 13:34:22 -0500 Subject: [PATCH 27/30] use title case for method names --- api.ts | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/api.ts b/api.ts index 984d3ce..3f650ca 100644 --- a/api.ts +++ b/api.ts @@ -775,7 +775,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async createStore(body: CreateStoreRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.createStore(body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "createStore", + [TelemetryAttribute.FgaClientRequestMethod]: "CreateStore", }); }, /** @@ -788,7 +788,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async deleteStore(storeId: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.deleteStore(storeId, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "deleteStore", + [TelemetryAttribute.FgaClientRequestMethod]: "DeleteStore", [TelemetryAttribute.FgaClientRequestStoreId]: storeId, }); }, @@ -803,7 +803,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async expand(storeId: string, body: ExpandRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.expand(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "expand", + [TelemetryAttribute.FgaClientRequestMethod]: "Expand", [TelemetryAttribute.FgaClientRequestModelId]: body.authorization_model_id ?? "", [TelemetryAttribute.FgaClientRequestStoreId]: storeId ?? "", }); @@ -818,7 +818,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async getStore(storeId: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.getStore(storeId, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "getStore", + [TelemetryAttribute.FgaClientRequestMethod]: "GetStore", [TelemetryAttribute.FgaClientRequestStoreId]: storeId, }); }, @@ -833,7 +833,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async listObjects(storeId: string, body: ListObjectsRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.listObjects(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "listObjects", + [TelemetryAttribute.FgaClientRequestMethod]: "ListObjects", [TelemetryAttribute.FgaClientRequestStoreId]: storeId ?? "", [TelemetryAttribute.FgaClientRequestModelId]: body.authorization_model_id ?? "", [TelemetryAttribute.FgaClientUser]: body.user @@ -850,7 +850,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async listStores(pageSize?: number, continuationToken?: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.listStores(pageSize, continuationToken, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "listStores", + [TelemetryAttribute.FgaClientRequestMethod]: "ListStores", }); }, /** @@ -864,7 +864,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async listUsers(storeId: string, body: ListUsersRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.listUsers(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "listUsers", + [TelemetryAttribute.FgaClientRequestMethod]: "ListUsers", [TelemetryAttribute.FgaClientRequestStoreId]: storeId ?? "", [TelemetryAttribute.FgaClientRequestModelId]: body.authorization_model_id ?? "", }); @@ -880,7 +880,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async read(storeId: string, body: ReadRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.read(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "read", + [TelemetryAttribute.FgaClientRequestMethod]: "Read", [TelemetryAttribute.FgaClientRequestStoreId]: storeId, }); }, @@ -895,7 +895,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async readAssertions(storeId: string, authorizationModelId: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.readAssertions(storeId, authorizationModelId, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "readAssertions", + [TelemetryAttribute.FgaClientRequestMethod]: "ReadAssertions", [TelemetryAttribute.FgaClientRequestStoreId]: storeId, [TelemetryAttribute.FgaClientRequestModelId]: authorizationModelId, }); @@ -911,7 +911,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async readAuthorizationModel(storeId: string, id: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.readAuthorizationModel(storeId, id, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "readAuthorizationModel", + [TelemetryAttribute.FgaClientRequestMethod]: "ReadAuthorizationModel", [TelemetryAttribute.FgaClientRequestStoreId]: storeId, }); }, @@ -927,7 +927,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async readAuthorizationModels(storeId: string, pageSize?: number, continuationToken?: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.readAuthorizationModels(storeId, pageSize, continuationToken, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "readAuthorizationModels", + [TelemetryAttribute.FgaClientRequestMethod]: "ReadAuthorizationModels", [TelemetryAttribute.FgaClientRequestStoreId]: storeId, }); }, @@ -944,7 +944,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async readChanges(storeId: string, type?: string, pageSize?: number, continuationToken?: string, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.readChanges(storeId, type, pageSize, continuationToken, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "readChanges", + [TelemetryAttribute.FgaClientRequestMethod]: "ReadChanges", [TelemetryAttribute.FgaClientRequestStoreId]: storeId, }); }, @@ -959,7 +959,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async write(storeId: string, body: WriteRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.write(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "write", + [TelemetryAttribute.FgaClientRequestMethod]: "Write", [TelemetryAttribute.FgaClientRequestStoreId]: storeId ?? "", [TelemetryAttribute.FgaClientRequestModelId]: body.authorization_model_id ?? "", }); @@ -976,7 +976,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async writeAssertions(storeId: string, authorizationModelId: string, body: WriteAssertionsRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.writeAssertions(storeId, authorizationModelId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "writeAssertions", + [TelemetryAttribute.FgaClientRequestMethod]: "WriteAssertions", [TelemetryAttribute.FgaClientRequestStoreId]: storeId, [TelemetryAttribute.FgaClientRequestModelId]: authorizationModelId, }); @@ -992,7 +992,7 @@ export const OpenFgaApiFp = function(configuration: Configuration, credentials: async writeAuthorizationModel(storeId: string, body: WriteAuthorizationModelRequest, options?: any): Promise<(axios?: AxiosInstance) => PromiseResult> { const localVarAxiosArgs = localVarAxiosParamCreator.writeAuthorizationModel(storeId, body, options); return createRequestFunction(localVarAxiosArgs, globalAxios, configuration, credentials, { - [TelemetryAttribute.FgaClientRequestMethod]: "writeAuthorizationModel", + [TelemetryAttribute.FgaClientRequestMethod]: "WriteAuthorizationModel", [TelemetryAttribute.FgaClientRequestStoreId]: storeId, }); }, From 1112a2c8ff26512b163cafb5f006bf93b96c5cf4 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Thu, 26 Sep 2024 18:23:12 -0500 Subject: [PATCH 28/30] fix to send all telemetry attributes --- common.ts | 34 ++++++++++++++++++++++-------- credentials/credentials.ts | 26 ++++++++++++++--------- docs/opentelemetry.md | 2 +- telemetry/attributes.ts | 16 ++++++-------- telemetry/configuration.ts | 2 +- tests/telemetry/attributes.test.ts | 14 ++---------- 6 files changed, 52 insertions(+), 42 deletions(-) diff --git a/common.ts b/common.ts index 1c23123..9a626ab 100644 --- a/common.ts +++ b/common.ts @@ -142,6 +142,11 @@ function randomTime(loopCount: number, minWaitInMs: number): number { return Math.floor(Math.random() * (max - min) + min); //The maximum is exclusive and the minimum is inclusive } +export interface WrappedAxiosResponse { + response?: AxiosResponse; + retries: number; +} + export async function attemptHttpRequest( request: AxiosRequestConfig, config: { @@ -149,12 +154,16 @@ export async function attemptHttpRequest( minWaitInMs: number; }, axiosInstance: AxiosInstance, -): Promise | undefined> { +): Promise | undefined> { let iterationCount = 0; do { iterationCount++; try { - return await axiosInstance(request); + const response = await axiosInstance(request); + return { + response: response, + retries: iterationCount - 1, + }; } catch (err: any) { if (!isAxiosError(err)) { throw new FgaError(err); @@ -194,17 +203,19 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst const maxRetry:number = retryParams ? retryParams.maxRetry : 0; const minWaitInMs:number = retryParams ? retryParams.minWaitInMs : 0; - const start = Date.now(); + const start = performance.now(); return async (axios: AxiosInstance = axiosInstance) : PromiseResult => { await setBearerAuthToObject(axiosArgs.options.headers, credentials!); - const axiosRequestArgs = {...axiosArgs.options, url: configuration.getBasePath() + axiosArgs.url}; - const response = await attemptHttpRequest(axiosRequestArgs, { + const url = configuration.getBasePath() + axiosArgs.url; + + const axiosRequestArgs = {...axiosArgs.options, url: url}; + const wrappedResponse = await attemptHttpRequest(axiosRequestArgs, { maxRetry, minWaitInMs, }, axios); - + const response = wrappedResponse?.response; const data = typeof response?.data === "undefined" ? {} : response?.data; const result: CallResult = { ...data }; setNotEnumerableProperty(result, "$response", response); @@ -212,6 +223,10 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst let attributes: StringIndexable = {}; attributes = TelemetryAttributes.fromRequest({ + userAgent: configuration.baseOptions?.headers["User-Agent"], + httpMethod: axiosArgs.options?.method, + url, + resendCount: wrappedResponse?.retries, start: start, credentials: credentials, attributes: methodAttributes, @@ -219,11 +234,12 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst attributes = TelemetryAttributes.fromResponse({ response, - credentials, attributes, }); - if (configuration.telemetry?.metrics?.histogramQueryDuration) { + // only if hisogramQueryDuration set AND if response header contains fga-query-duration-ms + const serverRequestDuration = attributes[TelemetryAttribute.HttpServerRequestDuration]; + if (configuration.telemetry?.metrics?.histogramQueryDuration && typeof serverRequestDuration !== "undefined") { configuration.telemetry.recorder.histogram( TelemetryHistograms.queryDuration, parseInt(attributes[TelemetryAttribute.HttpServerRequestDuration] as string, 10), @@ -237,7 +253,7 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst if (configuration.telemetry?.metrics?.histogramRequestDuration) { configuration.telemetry.recorder.histogram( TelemetryHistograms.requestDuration, - Date.now() - start, + attributes[TelemetryAttribute.HttpClientRequestDuration], TelemetryAttributes.prepare( attributes, configuration.telemetry.metrics.histogramRequestDuration.attributes diff --git a/credentials/credentials.ts b/credentials/credentials.ts index 289a49a..3156aa0 100644 --- a/credentials/credentials.ts +++ b/credentials/credentials.ts @@ -26,11 +26,11 @@ export class Credentials { private accessToken?: string; private accessTokenExpiryDate?: Date; - public static init(configuration: { credentials: AuthCredentialsConfig, telemetry: TelemetryConfiguration }): Credentials { - return new Credentials(configuration.credentials, globalAxios, configuration.telemetry); + public static init(configuration: { credentials: AuthCredentialsConfig, telemetry: TelemetryConfiguration, baseOptions?: any }): Credentials { + return new Credentials(configuration.credentials, globalAxios, configuration.telemetry, configuration.baseOptions); } - public constructor(private authConfig: AuthCredentialsConfig, private axios: AxiosInstance = globalAxios, private telemetryConfig: TelemetryConfiguration) { + public constructor(private authConfig: AuthCredentialsConfig, private axios: AxiosInstance = globalAxios, private telemetryConfig: TelemetryConfiguration, private baseOptions?: any) { this.initConfig(); this.isValid(); } @@ -129,9 +129,10 @@ export class Credentials { */ private async refreshAccessToken() { const clientCredentials = (this.authConfig as { method: CredentialsMethod.ClientCredentials; config: ClientCredentialsConfig })?.config; + const url = `https://${clientCredentials.apiTokenIssuer}/oauth/token`; try { - const response = await attemptHttpRequest<{ + const wrappedResponse = await attemptHttpRequest<{ client_id: string, client_secret: string, audience: string, @@ -140,8 +141,8 @@ export class Credentials { access_token: string, expires_in: number, }>({ - url: `https://${clientCredentials.apiTokenIssuer}/oauth/token`, - method: "post", + url, + method: "POST", data: { client_id: clientCredentials.clientId, client_secret: clientCredentials.clientSecret, @@ -156,6 +157,7 @@ export class Credentials { minWaitInMs: 100, }, globalAxios); + const response = wrappedResponse?.response; if (response) { this.accessToken = response.data.access_token; this.accessTokenExpiryDate = new Date(Date.now() + response.data.expires_in * 1000); @@ -166,18 +168,22 @@ export class Credentials { let attributes = {}; attributes = TelemetryAttributes.fromRequest({ + userAgent: this.baseOptions?.headers["User-Agent"], + fgaMethod: "TokenExchange", + url, + resendCount: wrappedResponse?.retries, + httpMethod: "POST", credentials: clientCredentials, - // resendCount: 0, // TODO: implement resend count tracking, not available in the current context + start: performance.now(), attributes, }); attributes = TelemetryAttributes.fromResponse({ response, - credentials: clientCredentials, - attributes, + attributes, }); - attributes = TelemetryAttributes.prepare(attributes); + attributes = TelemetryAttributes.prepare(attributes, this.telemetryConfig.metrics?.counterCredentialsRequest?.attributes); this.telemetryConfig.recorder.counter(TelemetryCounters.credentialsRequest, 1, attributes); } diff --git a/docs/opentelemetry.md b/docs/opentelemetry.md index ee83c7c..c259fcf 100644 --- a/docs/opentelemetry.md +++ b/docs/opentelemetry.md @@ -33,7 +33,7 @@ In cases when metrics events are sent, they will not be viewable outside of infr | `url.full` | `string` | No | The full URL of the request | | `url.scheme` | `string` | No | HTTP Scheme of the request (http/https) | | `http.request.resend_count` | `int` | Yes | The number of retries attempted | -| `http.client.request.duration` | `int` | No | Time taken by the FGA server to process and evaluate the request, in milliseconds | +| `http.client.request.duration` | `int` | No | Time taken by the FGA server to process and evaluate the request, rounded to the nearest milliseconds | | `http.server.request.duration` | `int` | No | The number of retries attempted | ## Default attributes diff --git a/telemetry/attributes.ts b/telemetry/attributes.ts index cba972e..dcc2129 100644 --- a/telemetry/attributes.ts +++ b/telemetry/attributes.ts @@ -67,7 +67,7 @@ export class TelemetryAttributes { attributes[TelemetryAttribute.UrlFull] = url; } - if (start) attributes[TelemetryAttribute.HttpClientRequestDuration] = Date.now() - start; + if (start) attributes[TelemetryAttribute.HttpClientRequestDuration] = Math.round(performance.now() - start); if (resendCount) attributes[TelemetryAttribute.HttpRequestResendCount] = resendCount; if (credentials && credentials.method === "client_credentials") { attributes[TelemetryAttribute.FgaClientRequestClientId] = credentials.configuration.clientId; @@ -78,24 +78,22 @@ export class TelemetryAttributes { static fromResponse({ response, - credentials, attributes = {}, }: { response: any; - credentials?: any; attributes?: Record; }): Record { if (response?.status) attributes[TelemetryAttribute.HttpResponseStatusCode] = response.status; const responseHeaders = response?.headers || {}; const responseModelId = responseHeaders["openfga-authorization-model-id"]; - const responseQueryDuration = responseHeaders["fga-query-duration-ms"]; - - if (responseModelId) attributes[TelemetryAttribute.FgaClientResponseModelId] = responseModelId; - if (responseQueryDuration) attributes[TelemetryAttribute.HttpServerRequestDuration] = responseQueryDuration; + const responseQueryDuration = responseHeaders["fga-query-duration-ms"] ? parseInt(responseHeaders["fga-query-duration-ms"], 10) : undefined; - if (credentials && credentials.method === "client_credentials") { - attributes[TelemetryAttribute.FgaClientRequestClientId] = credentials.configuration.clientId; + if (responseModelId) { + attributes[TelemetryAttribute.FgaClientResponseModelId] = responseModelId; + } + if (typeof responseQueryDuration !== "undefined" && Number.isFinite(responseQueryDuration)) { + attributes[TelemetryAttribute.HttpServerRequestDuration] = responseQueryDuration as number; } return attributes; diff --git a/telemetry/configuration.ts b/telemetry/configuration.ts index 9cb1767..505bf36 100644 --- a/telemetry/configuration.ts +++ b/telemetry/configuration.ts @@ -55,7 +55,7 @@ export class TelemetryConfiguration implements TelemetryConfig { // TelemetryAttribute.HttpRequestMethod, // TelemetryAttribute.UrlFull, // TelemetryAttribute.HttpClientRequestDuration, - // TelemetryAttribute.HttpServerRequestDuration + // TelemetryAttribute.HttpServerRequestDuration, // This not included by default as it has a very high cardinality which could increase costs for users // TelemetryAttribute.FgaClientUser diff --git a/tests/telemetry/attributes.test.ts b/tests/telemetry/attributes.test.ts index 87dd934..aa3559c 100644 --- a/tests/telemetry/attributes.test.ts +++ b/tests/telemetry/attributes.test.ts @@ -65,7 +65,7 @@ describe("TelemetryAttributes", () => { // Verify line 90 is covered - status is correctly set expect(result["http.response.status_code"]).toEqual(200); expect(result["fga-client.response.model_id"]).toEqual("model-id"); - expect(result["http.server.request.duration"]).toEqual("10"); + expect(result["http.server.request.duration"]).toEqual(10); }); test("should handle response without status correctly", () => { @@ -75,16 +75,6 @@ describe("TelemetryAttributes", () => { // Verify that no status code is set when response does not have a status expect(result["http.response.status_code"]).toBeUndefined(); expect(result["fga-client.response.model_id"]).toEqual("model-id"); - expect(result["http.server.request.duration"]).toEqual("10"); - }); - - test("should create attributes from response with client credentials", () => { - const response = { status: 200, headers: {} }; - const credentials = { method: "client_credentials", configuration: { clientId: "client-id" } }; - const result = TelemetryAttributes.fromResponse({ response, credentials }); - - // Check that the client ID is set correctly from the credentials - expect(result["http.response.status_code"]).toEqual(200); - expect(result["fga-client.request.client_id"]).toEqual("client-id"); + expect(result["http.server.request.duration"]).toEqual(10); }); }); From 8fb95c80ff57d10616c0fd60ca1dfc991ce41675 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Thu, 26 Sep 2024 21:03:34 -0500 Subject: [PATCH 29/30] don't expose wrapped response --- common.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common.ts b/common.ts index 9a626ab..be8b1aa 100644 --- a/common.ts +++ b/common.ts @@ -142,7 +142,7 @@ function randomTime(loopCount: number, minWaitInMs: number): number { return Math.floor(Math.random() * (max - min) + min); //The maximum is exclusive and the minimum is inclusive } -export interface WrappedAxiosResponse { +interface WrappedAxiosResponse { response?: AxiosResponse; retries: number; } From be94b36b63aac0056c14bb78ec3cd1447632bc24 Mon Sep 17 00:00:00 2001 From: Jim Anderson Date: Thu, 26 Sep 2024 21:11:15 -0500 Subject: [PATCH 30/30] fix typo --- telemetry/configuration.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/telemetry/configuration.ts b/telemetry/configuration.ts index 505bf36..25e046d 100644 --- a/telemetry/configuration.ts +++ b/telemetry/configuration.ts @@ -68,7 +68,7 @@ export class TelemetryConfiguration implements TelemetryConfig { * @readonly * @type {Set} */ - public static readonly validAttriburtes: Set = new Set([ + public static readonly validAttributes: Set = new Set([ TelemetryAttribute.HttpHost, TelemetryAttribute.HttpResponseStatusCode, TelemetryAttribute.UserAgentOriginal, @@ -115,7 +115,7 @@ export class TelemetryConfiguration implements TelemetryConfig { * @throws {FgaValidationError} Throws an error if any attribute in the metric configurations is invalid. */ public ensureValid(): void { - const validAttrs = TelemetryConfiguration.validAttriburtes; + const validAttrs = TelemetryConfiguration.validAttributes; const counterConfigAttrs = this.metrics?.counterCredentialsRequest?.attributes || new Set(); counterConfigAttrs.forEach(counterConfigAttr => {