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] Improve logic to determine when to use the transaction.duration.summary field #171315

Merged
merged 34 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
0de5219
Improve logic to determine when to use the transaction.duration.summa…
crespocarlos Nov 15, 2023
6a53f6d
Merge branch 'main' of github.com:elastic/kibana into 167578-empty-la…
crespocarlos Nov 15, 2023
c7f7683
Simplifying the solution
crespocarlos Nov 16, 2023
ec4177d
Small optimization
crespocarlos Nov 20, 2023
9a1d36f
Add test case
crespocarlos Nov 20, 2023
83e1c55
Merge branch 'main' into 167578-empty-latency-chart-fix
kibanamachine Nov 20, 2023
0e329c9
Fix after merge
crespocarlos Nov 20, 2023
0351b2f
Clean up
crespocarlos Nov 20, 2023
618cddc
CR fixes
crespocarlos Nov 23, 2023
0416fe9
Merge branch 'main' into 167578-empty-latency-chart-fix
kibanamachine Nov 23, 2023
72782f3
Verify when hasDurationSummaryField is true on a cluster level
crespocarlos Nov 23, 2023
bb2d2cd
Merge branch 'main' into 167578-empty-latency-chart-fix
kibanamachine Nov 27, 2023
a109347
Merge branch 'main' into 167578-empty-latency-chart-fix
kibanamachine Dec 1, 2023
72f76b0
Merge branch 'main' into 167578-empty-latency-chart-fix
kibanamachine Dec 12, 2023
ea01c51
Type fix
crespocarlos Dec 12, 2023
cf8a8f7
Merge branch 'main' into 167578-empty-latency-chart-fix
kibanamachine Dec 13, 2023
28a82ee
CR fixes
crespocarlos Dec 13, 2023
3c89c0f
Revert changes to base_client
crespocarlos Dec 13, 2023
1710bf6
Revert changes to base_client
crespocarlos Dec 13, 2023
aa2e343
Clean up
crespocarlos Dec 13, 2023
7a80eb7
More clean up
crespocarlos Dec 13, 2023
196724c
Merge branch 'main' into 167578-empty-latency-chart-fix
kibanamachine Dec 18, 2023
c5addd4
Merge branch 'main' into 167578-empty-latency-chart-fix
kibanamachine Dec 20, 2023
f89d228
CR fixes
crespocarlos Dec 21, 2023
0709537
Merge branch 'main' into 167578-empty-latency-chart-fix
kibanamachine Jan 2, 2024
0ee3106
[CI] Auto-commit changed files from 'node scripts/notice'
kibanamachine Jan 2, 2024
22d3e61
Refactor get_document_sources
crespocarlos Jan 3, 2024
ed45132
Small improvement
crespocarlos Jan 3, 2024
6b3f0bf
Clean up
crespocarlos Jan 3, 2024
33efa72
Merge branch 'main' into 167578-empty-latency-chart-fix
kibanamachine Jan 4, 2024
6d5eed5
Merge branch 'main' into 167578-empty-latency-chart-fix
kibanamachine Jan 8, 2024
f207b54
Merge branch 'main' into 167578-empty-latency-chart-fix
kibanamachine Jan 15, 2024
5897ee7
Rename variable
crespocarlos Jan 15, 2024
ecfa42c
Fix types
crespocarlos Jan 15, 2024
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
2 changes: 1 addition & 1 deletion NOTICE.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Kibana source code with Kibana X-Pack source code
Copyright 2012-2023 Elasticsearch B.V.
Copyright 2012-2024 Elasticsearch B.V.

---
Pretty handling of logarithmic axes.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* 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 { ApmFields, apm } from '@kbn/apm-synthtrace-client';
import { random } from 'lodash';
import { pipeline, Readable } from 'stream';
import semver from 'semver';
import { Scenario } from '../cli/scenario';
import {
addObserverVersionTransform,
deleteSummaryFieldTransform,
} from '../lib/utils/transform_helpers';
import { withClient } from '../lib/utils/with_client';

const scenario: Scenario<ApmFields> = async ({ logger, versionOverride }) => {
const version = versionOverride as string;
const isLegacy = versionOverride && semver.lt(version, '8.7.0');
return {
bootstrap: async ({ apmEsClient }) => {
if (isLegacy) {
apmEsClient.pipeline((base: Readable) => {
const defaultPipeline = apmEsClient.getDefaultPipeline()(
base
) as unknown as NodeJS.ReadableStream;

return pipeline(
defaultPipeline,
addObserverVersionTransform(version),
deleteSummaryFieldTransform(),
(err) => {
if (err) {
logger.error(err);
}
}
);
});
}
},
generate: ({ range, clients: { apmEsClient } }) => {
const successfulTimestamps = range.ratePerMinute(6);
const instance = apm
.service({
name: `java${isLegacy ? '-legacy' : ''}`,
environment: 'production',
agentName: 'java',
})
.instance(`instance`);

return withClient(
apmEsClient,
successfulTimestamps.generator((timestamp) => {
const randomHigh = random(1000, 4000);
const randomLow = random(100, randomHigh / 5);
const duration = random(randomLow, randomHigh);
return instance
.transaction({ transactionName: 'GET /order/{id}' })
.timestamp(timestamp)
.duration(duration)
.success();
})
);
},
};
};

export default scenario;
116 changes: 42 additions & 74 deletions x-pack/plugins/apm/server/lib/helpers/get_document_sources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import { ApmDocumentType } from '../../../common/document_type';
import { RollupInterval } from '../../../common/rollup';
import { APMEventClient } from './create_es_client/create_apm_event_client';
import { getConfigForDocumentType } from './create_es_client/document_type';
import { TRANSACTION_DURATION_SUMMARY } from '../../../common/es_fields/apm';
import { TimeRangeMetadata } from '../../../common/time_range_metadata';
import { getDurationLegacyFilter } from './transactions';

const NUMBER_OF_QUERIES = 3;

const getRequest = ({
documentType,
Expand Down Expand Up @@ -69,26 +71,24 @@ export async function getDocumentSources({
const diff = end - start;
const kql = kqlQuery(kuery);
const beforeRange = rangeQuery(start - diff, end - diff);

const sourcesToCheck = [
const documentTypesToCheck = [
...(enableServiceTransactionMetrics
? [ApmDocumentType.ServiceTransactionMetric as const]
: []),
ApmDocumentType.TransactionMetric as const,
].flatMap((documentType) => {
];

const sourcesToCheck = documentTypesToCheck.flatMap((documentType) => {
const docTypeConfig = getConfigForDocumentType(documentType);

return (
enableContinuousRollups
? docTypeConfig.rollupIntervals
: [RollupInterval.OneMinute]
).flatMap((rollupInterval) => {
).map((rollupInterval) => {
return {
documentType,
rollupInterval,
meta: {
checkSummaryFieldExists: false,
},
before: getRequest({
documentType,
rollupInterval,
Expand All @@ -99,77 +99,45 @@ export async function getDocumentSources({
rollupInterval,
filters: [...kql, ...currentRange],
}),
};
});
});

const sourcesToCheckWithSummary = [
ApmDocumentType.TransactionMetric as const,
].flatMap((documentType) => {
const docTypeConfig = getConfigForDocumentType(documentType);

return (
enableContinuousRollups
? docTypeConfig.rollupIntervals
: [RollupInterval.OneMinute]
).flatMap((rollupInterval) => {
const summaryExistsFilter = {
bool: {
filter: [
{
exists: {
field: TRANSACTION_DURATION_SUMMARY,
},
},
],
},
};

return {
documentType,
rollupInterval,
meta: {
checkSummaryFieldExists: true,
},
before: getRequest({
documentType,
rollupInterval,
filters: [...kql, ...beforeRange, summaryExistsFilter],
}),
current: getRequest({
durationSummaryCheck: getRequest({
Copy link
Member

Choose a reason for hiding this comment

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

this check is not necessary for service transaction metrics. can we avoid it?

documentType,
rollupInterval,
filters: [...kql, ...currentRange, summaryExistsFilter],
filters: [...kql, ...currentRange, getDurationLegacyFilter()],
}),
};
});
});

const allSourcesToCheck = [...sourcesToCheck, ...sourcesToCheckWithSummary];

const allSearches = allSourcesToCheck.flatMap(({ before, current }) => [
before,
current,
]);
const allSearches = [
...sourcesToCheck.flatMap(({ before, current, durationSummaryCheck }) => [
before,
current,
durationSummaryCheck,
]),
];

const allResponses = (
await apmEventClient.msearch('get_document_availability', ...allSearches)
).responses;

const checkedSources = allSourcesToCheck.map((source, index) => {
const checkedSources = sourcesToCheck.map((source, index) => {
const { documentType, rollupInterval } = source;
const responseBefore = allResponses[index * 2];
const responseAfter = allResponses[index * 2 + 1];
const responseBefore = allResponses[index * NUMBER_OF_QUERIES];
const responseAfter = allResponses[index * NUMBER_OF_QUERIES + 1];
const responseDurationSummaryCheck =
allResponses[index * NUMBER_OF_QUERIES + 2];

const hasDocBefore = responseBefore.hits.total.value > 0;
const hasDocAfter = responseAfter.hits.total.value > 0;
const hasDurationSummary =
responseDurationSummaryCheck.hits.total.value === 0;

return {
documentType,
rollupInterval,
hasDocBefore,
hasDocAfter,
checkSummaryFieldExists: source.meta.checkSummaryFieldExists,
hasDurationSummary,
};
});

Expand All @@ -183,46 +151,46 @@ export async function getDocumentSources({
hasDocAfter,
hasDocBefore,
rollupInterval,
checkSummaryFieldExists,
hasDurationSummary,
} = checkedSource;

const hasDocBeforeOrAfter = hasDocBefore || hasDocAfter;

// If there is any data before, we require that data is available before
// this time range to mark this source as available. If we don't do that,
// users that upgrade to a version that starts generating service tx metrics
// will see a mostly empty screen for a while after upgrading.
// If we only check before, users with a new deployment will use raw transaction
// events.
const hasDocs = hasAnySourceDocBefore ? hasDocBefore : hasDocBeforeOrAfter;

return {
documentType,
rollupInterval,
checkSummaryFieldExists,
hasDocs,
hasDurationSummary,
};
});

const sources: TimeRangeMetadata['sources'] = sourcesWithHasDocs
.filter((source) => !source.checkSummaryFieldExists)
.map((checkedSource) => {
const sources: TimeRangeMetadata['sources'] = sourcesWithHasDocs.map(
(checkedSource) => {
const { documentType, hasDocs, rollupInterval } = checkedSource;

return {
documentType,
rollupInterval,
hasDocs,
hasDurationSummaryField:
documentType === ApmDocumentType.ServiceTransactionMetric ||
Boolean(
sourcesWithHasDocs.find((eSource) => {
return (
eSource.documentType === documentType &&
eSource.rollupInterval === rollupInterval &&
eSource.checkSummaryFieldExists
);
})?.hasDocs
),
hasDurationSummaryField: Boolean(
sourcesWithHasDocs.find((eSource) => {
return (
eSource.documentType === documentType &&
eSource.rollupInterval === rollupInterval
);
})?.hasDurationSummary
),
};
});
}
);

return sources.concat({
documentType: ApmDocumentType.TransactionEvent,
Expand Down
35 changes: 30 additions & 5 deletions x-pack/plugins/apm/server/lib/helpers/transactions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import { kqlQuery, rangeQuery } from '@kbn/observability-plugin/server';
import { ProcessorEvent } from '@kbn/observability-plugin/common';
import { QueryDslQueryContainer } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { SearchAggregatedTransactionSetting } from '../../../../common/aggregated_transactions';
import {
TRANSACTION_DURATION,
Expand Down Expand Up @@ -90,13 +91,12 @@ export async function getSearchTransactionsEvents({
}
}

export function getDurationFieldForTransactions(
export function isSummaryFieldSupportedByDocType(
typeOrSearchAgggregatedTransactions:
| ApmDocumentType.ServiceTransactionMetric
| ApmDocumentType.TransactionMetric
| ApmDocumentType.TransactionEvent
| boolean,
useDurationSummaryField?: boolean
| boolean
) {
let type: ApmDocumentType;

Expand All @@ -108,10 +108,20 @@ export function getDurationFieldForTransactions(
type = typeOrSearchAgggregatedTransactions;
}

if (
return (
type === ApmDocumentType.ServiceTransactionMetric ||
type === ApmDocumentType.TransactionMetric
) {
);
}
export function getDurationFieldForTransactions(
typeOrSearchAgggregatedTransactions:
| ApmDocumentType.ServiceTransactionMetric
| ApmDocumentType.TransactionMetric
| ApmDocumentType.TransactionEvent
| boolean,
useDurationSummaryField?: boolean
) {
if (isSummaryFieldSupportedByDocType(typeOrSearchAgggregatedTransactions)) {
if (useDurationSummaryField) {
return TRANSACTION_DURATION_SUMMARY;
}
Expand Down Expand Up @@ -163,3 +173,18 @@ export function isRootTransaction(searchAggregatedTransactions: boolean) {
},
};
}

export function getDurationLegacyFilter(): QueryDslQueryContainer {
return {
bool: {
must: [
{
bool: {
filter: [{ exists: { field: TRANSACTION_DURATION_HISTOGRAM } }],
must_not: [{ exists: { field: TRANSACTION_DURATION_SUMMARY } }],
},
},
],
},
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,6 @@ export async function getServiceTransactionStats({
};
}) ?? [],
serviceOverflowCount:
response.aggregations?.sample?.overflowCount.value || 0,
response.aggregations?.sample?.overflowCount?.value || 0,
};
}
Loading