Skip to content

Commit

Permalink
Normalize URLs reported by RUM agent (elastic#80092)
Browse files Browse the repository at this point in the history
  • Loading branch information
joshdover authored Oct 13, 2020
1 parent f09aefb commit 286eb2a
Show file tree
Hide file tree
Showing 3 changed files with 235 additions and 6 deletions.
176 changes: 176 additions & 0 deletions src/core/public/apm_system.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

jest.mock('@elastic/apm-rum');
import { init, apm } from '@elastic/apm-rum';
import { ApmSystem } from './apm_system';

const initMock = init as jest.Mocked<typeof init>;
const apmMock = apm as DeeplyMockedKeys<typeof apm>;

describe('ApmSystem', () => {
afterEach(() => {
jest.resetAllMocks();
jest.resetAllMocks();
});

describe('setup', () => {
it('does not init apm if no config provided', async () => {
const apmSystem = new ApmSystem(undefined);
await apmSystem.setup();
expect(initMock).not.toHaveBeenCalled();
});

it('calls init with configuration', async () => {
const apmSystem = new ApmSystem({ active: true });
await apmSystem.setup();
expect(initMock).toHaveBeenCalledWith({ active: true });
});

it('adds globalLabels if provided', async () => {
const apmSystem = new ApmSystem({ active: true, globalLabels: { alpha: 'one' } });
await apmSystem.setup();
expect(apm.addLabels).toHaveBeenCalledWith({ alpha: 'one' });
});

describe('http request normalization', () => {
let windowSpy: any;

beforeEach(() => {
windowSpy = jest.spyOn(global as any, 'window', 'get').mockImplementation(() => ({
location: {
protocol: 'http:',
hostname: 'mykibanadomain.com',
port: '5601',
},
}));
});

afterEach(() => {
windowSpy.mockRestore();
});

it('adds an observe function', async () => {
const apmSystem = new ApmSystem({ active: true });
await apmSystem.setup();
expect(apm.observe).toHaveBeenCalledWith('transaction:end', expect.any(Function));
});

/**
* Utility function to wrap functions that mutate their input but don't return the mutated value.
* Makes expects easier below.
*/
const returnArg = <T>(func: (input: T) => any): ((input: T) => T) => {
return (input) => {
func(input);
return input;
};
};

it('removes the hostname, port, and protocol only when all match window.location', async () => {
const apmSystem = new ApmSystem({ active: true });
await apmSystem.setup();
const observer = apmMock.observe.mock.calls[0][1];
const wrappedObserver = returnArg(observer);

// Strips the hostname, protocol, and port from URLs that are on the same origin
expect(
wrappedObserver({
type: 'http-request',
name: 'GET http://mykibanadomain.com:5601/asdf/qwerty',
} as Transaction)
).toEqual({ type: 'http-request', name: 'GET /asdf/qwerty' });

// Does not modify URLs that are not on the same origin
expect(
wrappedObserver({
type: 'http-request',
name: 'GET https://mykibanadomain.com:5601/asdf/qwerty',
} as Transaction)
).toEqual({
type: 'http-request',
name: 'GET https://mykibanadomain.com:5601/asdf/qwerty',
});

expect(
wrappedObserver({
type: 'http-request',
name: 'GET http://mykibanadomain.com:9200/asdf/qwerty',
} as Transaction)
).toEqual({
type: 'http-request',
name: 'GET http://mykibanadomain.com:9200/asdf/qwerty',
});

expect(
wrappedObserver({
type: 'http-request',
name: 'GET http://myotherdomain.com:5601/asdf/qwerty',
} as Transaction)
).toEqual({
type: 'http-request',
name: 'GET http://myotherdomain.com:5601/asdf/qwerty',
});
});

it('strips the basePath', async () => {
const apmSystem = new ApmSystem({ active: true }, '/alpha');
await apmSystem.setup();
const observer = apmMock.observe.mock.calls[0][1];
const wrappedObserver = returnArg(observer);

expect(
wrappedObserver({
type: 'http-request',
name: 'GET http://mykibanadomain.com:5601/alpha',
} as Transaction)
).toEqual({ type: 'http-request', name: 'GET /' });

expect(
wrappedObserver({
type: 'http-request',
name: 'GET http://mykibanadomain.com:5601/alpha/',
} as Transaction)
).toEqual({ type: 'http-request', name: 'GET /' });

expect(
wrappedObserver({
type: 'http-request',
name: 'GET http://mykibanadomain.com:5601/alpha/beta',
} as Transaction)
).toEqual({ type: 'http-request', name: 'GET /beta' });

expect(
wrappedObserver({
type: 'http-request',
name: 'GET http://mykibanadomain.com:5601/alpha/beta/',
} as Transaction)
).toEqual({ type: 'http-request', name: 'GET /beta/' });

// Works with relative URLs as well
expect(
wrappedObserver({
type: 'http-request',
name: 'GET /alpha/beta/',
} as Transaction)
).toEqual({ type: 'http-request', name: 'GET /beta/' });
});
});
});
});
63 changes: 58 additions & 5 deletions src/core/public/apm_system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,17 @@
* under the License.
*/

import type { ApmBase } from '@elastic/apm-rum';
import { modifyUrl } from '@kbn/std';
import type { InternalApplicationStart } from './application';

/** "GET protocol://hostname:port/pathname" */
const HTTP_REQUEST_TRANSACTION_NAME_REGEX = /^(GET|POST|PUT|HEAD|PATCH|DELETE|OPTIONS|CONNECT|TRACE)\s(.*)$/;

/**
* This is the entry point used to boot the frontend when serving a application
* that lives in the Kibana Platform.
*
* Any changes to this file should be kept in sync with
* src/legacy/ui/ui_bundles/app_entry_template.js
*/
import type { InternalApplicationStart } from './application';

interface ApmConfig {
// AgentConfigOptions is not exported from @elastic/apm-rum
Expand All @@ -42,7 +45,7 @@ export class ApmSystem {
* `apmConfig` would be populated with relevant APM RUM agent
* configuration if server is started with elastic.apm.* config.
*/
constructor(private readonly apmConfig?: ApmConfig) {
constructor(private readonly apmConfig?: ApmConfig, private readonly basePath = '') {
this.enabled = apmConfig != null && !!apmConfig.active;
}

Expand All @@ -54,6 +57,8 @@ export class ApmSystem {
apm.addLabels(globalLabels);
}

this.addHttpRequestNormalization(apm);

init(apmConfig);
}

Expand All @@ -73,4 +78,52 @@ export class ApmSystem {
}
});
}

/**
* Adds an observer to the APM configuration for normalizing transactions of the 'http-request' type to remove the
* hostname, protocol, port, and base path. Allows for coorelating data cross different deployments.
*/
private addHttpRequestNormalization(apm: ApmBase) {
apm.observe('transaction:end', (t) => {
if (t.type !== 'http-request') {
return;
}

/** Split URLs of the from "GET protocol://hostname:port/pathname" into method & hostname */
const matches = t.name.match(HTTP_REQUEST_TRANSACTION_NAME_REGEX);
if (!matches) {
return;
}

const [, method, originalUrl] = matches;
// Normalize the URL
const normalizedUrl = modifyUrl(originalUrl, (parts) => {
const isAbsolute = parts.hostname && parts.protocol && parts.port;
// If the request was to a different host, port, or protocol then don't change anything
if (
isAbsolute &&
(parts.hostname !== window.location.hostname ||
parts.protocol !== window.location.protocol ||
parts.port !== window.location.port)
) {
return;
}

// Strip the protocol, hostnname, port, and protocol slashes to normalize
parts.protocol = null;
parts.hostname = null;
parts.port = null;
parts.slashes = false;

// Replace the basePath if present in the pathname
if (parts.pathname === this.basePath) {
parts.pathname = '/';
} else if (parts.pathname?.startsWith(`${this.basePath}/`)) {
parts.pathname = parts.pathname?.slice(this.basePath.length);
}
});

t.name = `${method} ${normalizedUrl}`;
});
}
}
2 changes: 1 addition & 1 deletion src/core/public/kbn_bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export async function __kbnBootstrap__() {
);

let i18nError: Error | undefined;
const apmSystem = new ApmSystem(injectedMetadata.vars.apmConfig);
const apmSystem = new ApmSystem(injectedMetadata.vars.apmConfig, injectedMetadata.basePath);

await Promise.all([
// eslint-disable-next-line no-console
Expand Down

0 comments on commit 286eb2a

Please sign in to comment.