Skip to content

Commit

Permalink
make telemetry attributes methods static
Browse files Browse the repository at this point in the history
  • Loading branch information
jimmyjames committed Sep 19, 2024
1 parent 6a78ecb commit 30da994
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 27 deletions.
10 changes: 5 additions & 5 deletions common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,18 +209,18 @@ export const createRequestFunction = function (axiosArgs: RequestArgs, axiosInst
const result: CallResult<any> = { ...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,
Expand All @@ -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
)
Expand All @@ -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
)
Expand Down
9 changes: 4 additions & 5 deletions credentials/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
6 changes: 3 additions & 3 deletions telemetry/attributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export enum TelemetryAttribute {
}

export class TelemetryAttributes {
prepare(
static prepare(
attributes?: Record<string, string | number>,
filter?: Set<TelemetryAttribute>
): Record<string, string | number> {
Expand All @@ -38,7 +38,7 @@ export class TelemetryAttributes {
return result;
}

fromRequest({
static fromRequest({
userAgent,
fgaMethod,
httpMethod,
Expand Down Expand Up @@ -77,7 +77,7 @@ export class TelemetryAttributes {
return attributes;
}

fromResponse({
static fromResponse({
response,
credentials,
attributes = {},
Expand Down
21 changes: 8 additions & 13 deletions tests/telemetry/attributes.test.ts
Original file line number Diff line number Diff line change
@@ -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 = {
Expand All @@ -14,7 +9,7 @@ describe("TelemetryAttributes", () => {
};

const filter = new Set<TelemetryAttribute>([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" });
});
Expand All @@ -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>([
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", () => {
Expand All @@ -42,11 +37,11 @@ describe("TelemetryAttributes", () => {
const filter = new Set<TelemetryAttribute>([
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",
Expand All @@ -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);
Expand All @@ -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();
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion tests/telemetry/metrics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 30da994

Please sign in to comment.