Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: client identification headers #690

Merged
merged 2 commits into from
Jan 8, 2025
Merged

feat: client identification headers #690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
130ee63
feat: client identification headers
kwasniew Jan 7, 2025
5c7ae36
chore: more explicit test names
kwasniew Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/metrics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { SUPPORTED_SPEC_VERSION } from './repository';
export interface MetricsOptions {
appName: string;
instanceId: string;
connectionId: string;
strategies: string[];
metricsInterval: number;
metricsJitter?: number;
Expand Down Expand Up @@ -80,6 +81,8 @@ export default class Metrics extends EventEmitter {

private instanceId: string;

private connectionId: string;

private sdkVersion: string;

private strategies: string[];
Expand Down Expand Up @@ -111,6 +114,7 @@ export default class Metrics extends EventEmitter {
constructor({
appName,
instanceId,
connectionId,
strategies,
metricsInterval = 0,
metricsJitter = 0,
Expand All @@ -127,6 +131,7 @@ export default class Metrics extends EventEmitter {
this.metricsJitter = metricsJitter;
this.appName = appName;
this.instanceId = instanceId;
this.connectionId = connectionId;
this.sdkVersion = sdkVersion;
this.strategies = strategies;
this.url = url;
Expand Down Expand Up @@ -198,6 +203,7 @@ export default class Metrics extends EventEmitter {
json: payload,
appName: this.appName,
instanceId: this.instanceId,
connectionId: this.connectionId,
headers,
timeout: this.timeout,
httpOptions: this.httpOptions,
Expand Down Expand Up @@ -250,6 +256,7 @@ export default class Metrics extends EventEmitter {
json: payload,
appName: this.appName,
instanceId: this.instanceId,
connectionId: this.connectionId,
headers,
timeout: this.timeout,
httpOptions: this.httpOptions,
Expand Down
8 changes: 7 additions & 1 deletion src/repository/bootstrap-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,13 @@ export class DefaultBootstrapProvider implements BootstrapProvider {
const response = await fetch(bootstrapUrl, {
method: 'GET',
timeout: 10_000,
headers: buildHeaders(this.appName, this.instanceId, undefined, undefined, this.urlHeaders),
headers: buildHeaders({
appName: this.appName,
instanceId: this.instanceId,
etag: undefined,
contentType: undefined,
custom: this.urlHeaders,
}),
retry: {
retries: 2,
maxTimeout: 10_000,
Expand Down
6 changes: 6 additions & 0 deletions src/repository/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export interface RepositoryOptions {
url: string;
appName: string;
instanceId: string;
connectionId: string;
projectName?: string;
refreshInterval: number;
timeout?: number;
Expand Down Expand Up @@ -59,6 +60,8 @@ export default class Repository extends EventEmitter implements EventEmitter {

private instanceId: string;

private connectionId: string;

private refreshInterval: number;

private headers?: CustomHeaders;
Expand Down Expand Up @@ -101,6 +104,7 @@ export default class Repository extends EventEmitter implements EventEmitter {
url,
appName,
instanceId,
connectionId,
projectName,
refreshInterval = 15_000,
timeout,
Expand All @@ -118,6 +122,7 @@ export default class Repository extends EventEmitter implements EventEmitter {
this.url = url;
this.refreshInterval = refreshInterval;
this.instanceId = instanceId;
this.connectionId = connectionId;
this.appName = appName;
this.projectName = projectName;
this.headers = headers;
Expand Down Expand Up @@ -386,6 +391,7 @@ Message: ${err.message}`,
appName: this.appName,
timeout: this.timeout,
instanceId: this.instanceId,
connectionId: this.connectionId,
headers,
httpOptions: this.httpOptions,
supportedSpecVersion: SUPPORTED_SPEC_VERSION,
Expand Down
59 changes: 49 additions & 10 deletions src/request.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { URL } from 'url';
import { getProxyForUrl } from 'proxy-from-env';
import { CustomHeaders } from './headers';
import { HttpOptions } from './http-options';
const details = require('./details.json');

export interface RequestOptions {
url: string;
Expand All @@ -18,6 +19,7 @@ export interface GetRequestOptions extends RequestOptions {
etag?: string;
appName?: string;
instanceId?: string;
connectionId: string;
supportedSpecVersion?: string;
httpOptions?: HttpOptions;
}
Expand All @@ -30,6 +32,7 @@ export interface PostRequestOptions extends RequestOptions {
json: Data;
appName?: string;
instanceId?: string;
connectionId?: string;
httpOptions?: HttpOptions;
}

Expand All @@ -55,18 +58,35 @@ export const getDefaultAgent = (url: URL) => {
: new HttpProxyAgent(proxy, httpAgentOptions);
};

export const buildHeaders = (
appName?: string,
instanceId?: string,
etag?: string,
contentType?: string,
custom?: CustomHeaders,
specVersionSupported?: string,
): Record<string, string> => {
type HeaderOptions = {
appName?: string;
instanceId?: string;
etag?: string;
contentType?: string;
custom?: CustomHeaders;
specVersionSupported?: string;
connectionId?: string;
};

export const buildHeaders = ({
appName,
instanceId,
etag,
contentType,
custom,
specVersionSupported,
connectionId,
}: HeaderOptions): Record<string, string> => {
const head: Record<string, string> = {};
if (appName) {
// TODO: delete
head['UNLEASH-APPNAME'] = appName;
// TODO: delete
Comment on lines +82 to +84
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete when, though? Next major?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either next major or at least another PR to avoid expanding and contracting headers in the same one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to have a super safe version upgrade that I can recommend to everyone even if for some unknown reason they relied on he old headers

head['User-Agent'] = appName;
head['x-unleash-appname'] = appName;
}
if (connectionId) {
head['x-unleash-connection-id'] = connectionId;
}
if (instanceId) {
head['UNLEASH-INSTANCEID'] = instanceId;
Expand All @@ -80,6 +100,8 @@ export const buildHeaders = (
if (specVersionSupported) {
head['Unleash-Client-Spec'] = specVersionSupported;
}
const version = details.version;
head['x-unleash-sdk'] = `unleash-node@${version}`;
if (custom) {
Object.assign(head, custom);
}
Expand All @@ -91,6 +113,7 @@ export const post = ({
appName,
timeout,
instanceId,
connectionId,
headers,
json,
httpOptions,
Expand All @@ -99,7 +122,14 @@ export const post = ({
timeout: timeout || 10000,
method: 'POST',
agent: httpOptions?.agent || getDefaultAgent,
headers: buildHeaders(appName, instanceId, undefined, 'application/json', headers),
headers: buildHeaders({
appName,
instanceId,
connectionId,
etag: undefined,
contentType: 'application/json',
custom: headers,
}),
body: JSON.stringify(json),
strictSSL: httpOptions?.rejectUnauthorized,
});
Expand All @@ -110,6 +140,7 @@ export const get = ({
appName,
timeout,
instanceId,
connectionId,
headers,
httpOptions,
supportedSpecVersion,
Expand All @@ -118,7 +149,15 @@ export const get = ({
method: 'GET',
timeout: timeout || 10_000,
agent: httpOptions?.agent || getDefaultAgent,
headers: buildHeaders(appName, instanceId, etag, undefined, headers, supportedSpecVersion),
headers: buildHeaders({
appName,
instanceId,
etag,
contentType: undefined,
custom: headers,
specVersionSupported: supportedSpecVersion,
connectionId,
}),
retry: {
retries: 2,
maxTimeout: timeout || 10_000,
Expand Down
22 changes: 19 additions & 3 deletions src/test/metrics.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,17 +122,27 @@ test('should sendMetrics', async (t) => {
t.true(metricsEP.isDone());
});

test('should send custom headers', (t) =>
test('should send correct custom and x-unleash headers', (t) =>
new Promise((resolve) => {
const url = getUrl();
t.plan(2);
const randomKey = `value-${Math.random()}`;
const metricsEP = nockMetrics(url).matchHeader('randomKey', randomKey);
const regEP = nockRegister(url).matchHeader('randomKey', randomKey);
const metricsEP = nockMetrics(url)
.matchHeader('randomKey', randomKey)
.matchHeader('x-unleash-appname', 'appName')
.matchHeader('x-unleash-sdk', /^unleash-node@(\d+\.\d+\.\d+)$/)
.matchHeader('x-unleash-connection-id', 'connectionId');
const regEP = nockRegister(url)
.matchHeader('randomKey', randomKey)
.matchHeader('x-unleash-appname', 'appName')
.matchHeader('x-unleash-sdk', /^unleash-node@(\d+\.\d+\.\d+)$/)
.matchHeader('x-unleash-connection-id', 'connectionId');

// @ts-expect-error
const metrics = new Metrics({
url,
appName: 'appName',
connectionId: 'connectionId',
metricsInterval: 50,
headers: {
randomKey,
Expand Down Expand Up @@ -259,6 +269,7 @@ test('sendMetrics should backoff on 404', async (t) => {
const metrics = new Metrics({
appName: '404-tester',
instanceId: '404-instance',
connectionId: '404-connection',
metricsInterval: 10,
strategies: [],
url,
Expand Down Expand Up @@ -449,6 +460,7 @@ test('sendMetrics should stop on 401', async (t) => {
const metrics = new Metrics({
appName: '401-tester',
instanceId: '401-instance',
connectionId: '401-connection',
metricsInterval: 0,
strategies: [],
url,
Expand All @@ -465,6 +477,7 @@ test('sendMetrics should stop on 403', async (t) => {
const metrics = new Metrics({
appName: '401-tester',
instanceId: '401-instance',
connectionId: '401-connection',
metricsInterval: 0,
strategies: [],
url,
Expand All @@ -481,6 +494,7 @@ test('sendMetrics should backoff on 429', async (t) => {
const metrics = new Metrics({
appName: '429-tester',
instanceId: '429-instance',
connectionId: '429-connection',
metricsInterval: 10,
strategies: [],
url,
Expand All @@ -504,6 +518,7 @@ test('sendMetrics should backoff on 500', async (t) => {
const metrics = new Metrics({
appName: '500-tester',
instanceId: '500-instance',
connectionId: '500-connetion',
metricsInterval: 10,
strategies: [],
url,
Expand All @@ -528,6 +543,7 @@ test('sendMetrics should backoff on 429 and gradually reduce interval', async (t
const metrics = new Metrics({
appName: '429-tester',
instanceId: '429-instance',
connectionId: '429-connection',
metricsInterval,
strategies: [],
url,
Expand Down
Loading
Loading