Skip to content

Commit

Permalink
fix to send all telemetry attributes
Browse files Browse the repository at this point in the history
  • Loading branch information
jimmyjames committed Sep 26, 2024
1 parent 33c98b8 commit 1112a2c
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 42 deletions.
34 changes: 25 additions & 9 deletions common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,19 +142,28 @@ 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<R> {
response?: AxiosResponse<R>;
retries: number;
}

export async function attemptHttpRequest<B, R>(
request: AxiosRequestConfig<B>,
config: {
maxRetry: number;
minWaitInMs: number;
},
axiosInstance: AxiosInstance,
): Promise<AxiosResponse<R> | undefined> {
): Promise<WrappedAxiosResponse<R> | 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);
Expand Down Expand Up @@ -194,36 +203,43 @@ 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<any> => {
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<any> = { ...data };
setNotEnumerableProperty(result, "$response", response);

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,
});

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),
Expand All @@ -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
Expand Down
26 changes: 16 additions & 10 deletions credentials/credentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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);
Expand All @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion docs/opentelemetry.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 7 additions & 9 deletions telemetry/attributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -78,24 +78,22 @@ export class TelemetryAttributes {

static fromResponse({
response,
credentials,
attributes = {},
}: {
response: any;
credentials?: any;
attributes?: Record<string, string | number>;
}): Record<string, string | number> {
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;
Expand Down
2 changes: 1 addition & 1 deletion telemetry/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 2 additions & 12 deletions tests/telemetry/attributes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand All @@ -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);
});
});

0 comments on commit 1112a2c

Please sign in to comment.