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.

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

[APM] Add custom spans around async operations #90403

Merged
merged 21 commits into from
Feb 12, 2021
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
3a48576
[APM] Add custom spans around async operations
dgieselaar Feb 5, 2021
5bee718
Merge branch 'master' of github.com:elastic/kibana into with-span
dgieselaar Feb 5, 2021
69cff93
License change
dgieselaar Feb 5, 2021
96a5ac9
Clarify setting activeSpan with a comment
dgieselaar Feb 5, 2021
559c1c4
Merge branch 'master' of github.com:elastic/kibana into with-span
dgieselaar Feb 5, 2021
0e57181
Move withSpan to separate package
dgieselaar Feb 5, 2021
af98d66
Merge branch 'master' of github.com:elastic/kibana into with-span
dgieselaar Feb 5, 2021
8995793
Merge branch 'master' into with-span
kibanamachine Feb 8, 2021
6506d2a
Merge branch 'master' of github.com:elastic/kibana into with-span
dgieselaar Feb 9, 2021
4283e72
Rename with_span to with_apm_span
dgieselaar Feb 9, 2021
66ba699
Merge branch 'with-span' of github.com:dgieselaar/kibana into with-span
dgieselaar Feb 9, 2021
5f56ed7
Merge branch 'master' into with-span
kibanamachine Feb 9, 2021
722e910
Merge branch 'master' of github.com:elastic/kibana into with-span
dgieselaar Feb 11, 2021
6da51cc
Merge branch 'with-span' of github.com:dgieselaar/kibana into with-sp…
dgieselaar Feb 11, 2021
927a4c4
Instrument remaining routes
dgieselaar Feb 11, 2021
06a899d
Merge branch 'master' of github.com:elastic/kibana into with-span
dgieselaar Feb 11, 2021
295fabb
Typo
dgieselaar Feb 11, 2021
43d7833
Merge branch 'master' of github.com:elastic/kibana into with-span
dgieselaar Feb 11, 2021
c36b8d8
Merge branch 'master' of github.com:elastic/kibana into with-span
dgieselaar Feb 12, 2021
ec3db1c
Typo
dgieselaar Feb 12, 2021
4c1bc7c
Await Promise.resolve() to prevent ended spans from creating children
dgieselaar Feb 12, 2021
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: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@
"@kbn/ace": "link:packages/kbn-ace",
"@kbn/analytics": "link:packages/kbn-analytics",
"@kbn/apm-config-loader": "link:packages/kbn-apm-config-loader",
"@kbn/apm-utils": "link:packages/kbn-apm-utils",
"@kbn/config": "link:packages/kbn-config",
"@kbn/config-schema": "link:packages/kbn-config-schema",
"@kbn/i18n": "link:packages/kbn-i18n",
Expand All @@ -129,6 +130,7 @@
"@kbn/logging": "link:packages/kbn-logging",
"@kbn/monaco": "link:packages/kbn-monaco",
"@kbn/std": "link:packages/kbn-std",
"@kbn/tinymath": "link:packages/kbn-tinymath",
"@kbn/ui-framework": "link:packages/kbn-ui-framework",
"@kbn/ui-shared-deps": "link:packages/kbn-ui-shared-deps",
"@kbn/utils": "link:packages/kbn-utils",
Expand Down Expand Up @@ -312,7 +314,6 @@
"tabbable": "1.1.3",
"tar": "4.4.13",
"tinygradient": "0.4.3",
"@kbn/tinymath": "link:packages/kbn-tinymath",
"tree-kill": "^1.2.2",
"ts-easing": "^0.2.0",
"tslib": "^2.0.0",
Expand Down Expand Up @@ -390,10 +391,10 @@
"@scant/router": "^0.1.0",
"@storybook/addon-a11y": "^6.0.26",
"@storybook/addon-actions": "^6.0.26",
"@storybook/addon-docs": "^6.0.26",
"@storybook/addon-essentials": "^6.0.26",
"@storybook/addon-knobs": "^6.0.26",
"@storybook/addon-storyshots": "^6.0.26",
"@storybook/addon-docs": "^6.0.26",
"@storybook/components": "^6.0.26",
"@storybook/core": "^6.0.26",
"@storybook/core-events": "^6.0.26",
Expand Down Expand Up @@ -851,4 +852,4 @@
"yargs": "^15.4.1",
"zlib": "^1.0.5"
}
}
}
13 changes: 13 additions & 0 deletions packages/kbn-apm-utils/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "@kbn/apm-utils",
"main": "./target/index.js",
"types": "./target/index.d.ts",
"version": "1.0.0",
"license": "SSPL-1.0 OR Elastic License 2.0",
"private": true,
"scripts": {
"build": "../../node_modules/.bin/tsc",
"kbn:bootstrap": "yarn build",
"kbn:watch": "yarn build --watch"
}
}
75 changes: 75 additions & 0 deletions packages/kbn-apm-utils/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import agent from 'elastic-apm-node';
import asyncHooks from 'async_hooks';

export interface SpanOptions {
name: string;
type?: string;
subtype?: string;
labels?: Record<string, string>;
}

export function parseSpanOptions(optionsOrName: SpanOptions | string) {
const options = typeof optionsOrName === 'string' ? { name: optionsOrName } : optionsOrName;

return options;
}

export function withSpan<T>(optionsOrName: SpanOptions | string, cb: () => Promise<T>): Promise<T> {
const options = parseSpanOptions(optionsOrName);

const { name, type, subtype, labels } = options;

if (!agent.isStarted()) {
return cb() as any;
}

const span = agent.startSpan(name);

if (!span) {
return cb() as any;
}

const resource = new asyncHooks.AsyncResource('fake_async');

return resource.runInAsyncScope(() => {
Copy link
Member

Choose a reason for hiding this comment

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

Pretty sure this would work for promise based cb, Did you happen to investigate if this wrapper works for callback based functions as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

What do you mean?

// set the active span for the newly created async context
// @ts-ignore
agent._instrumentation.activeSpan = span;
if (type) {
span.type = type;
}
if (subtype) {
span.subtype = subtype;
}

if (labels) {
span.addLabels(labels);
}

const promise = cb();

return promise
.then(
(res) => {
span.outcome = 'success';
Copy link
Member

Choose a reason for hiding this comment

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

hmmm not sure about this one. As outcome changes based on status code and the code here is based on the promise being resolved or rejected. Would double check with the current behaviour.

Copy link
Member

Choose a reason for hiding this comment

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

To me this looks ok.

As outcome changes based on status code and the code here is based on the promise being resolved or rejected.

Isn't it rather: "transaction outcome changes based on status code."

So a transaction outcome doesn't depend on span outcome and vice versa. If a span fails it will only also fail the transaction if the error is not caught. Conversely, if a span succeeds, any other span could fail, causing the transaction to fail too.

Copy link
Member Author

@dgieselaar dgieselaar Feb 9, 2021

Choose a reason for hiding this comment

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

spans can also result in http requests, for instance when calling an external service (ie, spans can have outcomes based on status code as well). But this function wraps async operations, and it is the responsibility of that operation to reject or resolve based on its interpretation of the results. Interpreting requests is out of scope for this function.

return res;
},
(err) => {
span.outcome = 'failure';
throw err;
}
)
.finally(() => {
span.end();
resource.emitDestroy();
});
});
}
18 changes: 18 additions & 0 deletions packages/kbn-apm-utils/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"declaration": true,
"outDir": "./target",
"stripInternal": false,
"declarationMap": true,
"types": [
"node"
]
},
"include": [
"./src/**/*.ts"
],
"exclude": [
"target"
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,23 @@
*/

import { MlPluginSetup } from '../../../../ml/server';
import { withApmSpan } from '../../utils/with_apm_span';
import { APM_ML_JOB_GROUP } from './constants';

// returns ml jobs containing "apm" group
// workaround: the ML api returns 404 when no jobs are found. This is handled so instead of throwing an empty response is returned
export async function getMlJobsWithAPMGroup(
export function getMlJobsWithAPMGroup(
anomalyDetectors: ReturnType<MlPluginSetup['anomalyDetectorsProvider']>
) {
try {
return await anomalyDetectors.jobs(APM_ML_JOB_GROUP);
} catch (e) {
if (e.statusCode === 404) {
return { count: 0, jobs: [] };
}
return withApmSpan('get_ml_jobs_with_apm_group', async () => {
try {
return await anomalyDetectors.jobs(APM_ML_JOB_GROUP);
} catch (e) {
if (e.statusCode === 404) {
return { count: 0, jobs: [] };
}

throw e;
}
throw e;
}
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import { ESFilter } from '../../../../../../typings/elasticsearch';
import { TRANSACTION_DURATION } from '../../../../common/elasticsearch_fieldnames';
import { ProcessorEvent } from '../../../../common/processor_event';
import { withApmSpan } from '../../../utils/with_apm_span';
import { Setup, SetupTimeRange } from '../../helpers/setup_request';

export async function getDurationForPercentile({
Expand All @@ -19,26 +20,28 @@ export async function getDurationForPercentile({
backgroundFilters: ESFilter[];
setup: Setup & SetupTimeRange;
}) {
const { apmEventClient } = setup;
const res = await apmEventClient.search({
apm: {
events: [ProcessorEvent.transaction],
},
body: {
size: 0,
query: {
bool: { filter: backgroundFilters },
return withApmSpan('get_duration_for_percentiles', async () => {
const { apmEventClient } = setup;
const res = await apmEventClient.search({
apm: {
events: [ProcessorEvent.transaction],
},
aggs: {
percentile: {
percentiles: {
field: TRANSACTION_DURATION,
percents: [durationPercentile],
body: {
size: 0,
query: {
bool: { filter: backgroundFilters },
},
aggs: {
percentile: {
percentiles: {
field: TRANSACTION_DURATION,
percents: [durationPercentile],
},
},
},
},
},
});
});

return Object.values(res.aggregations?.percentile.values || {})[0];
return Object.values(res.aggregations?.percentile.values || {})[0];
});
}
Loading