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

[APM UI] Refactor to use @timestamp and make timestamp.us optional as fallback value #194100

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
+75 −14
Draft

[APM UI] Refactor to use @timestamp and make timestamp.us optional as fallback value #194100

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
72db686
refactor: make timestamp.us optional and try to use @timestamp
rmyz Sep 26, 2024
e74b97e
refactor: keep microseconds precision
rmyz Sep 26, 2024
678d41e
refactor: update naming
rmyz Sep 26, 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
3 changes: 2 additions & 1 deletion packages/kbn-apm-types/src/es_fields/apm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/

export const TIMESTAMP = 'timestamp.us';
export const TIMESTAMP = '@timestamp';
export const FALLBACK_TIMESTAMP = 'timestamp.us';
export const AGENT = 'agent';
export const AGENT_NAME = 'agent.name';
export const AGENT_VERSION = 'agent.version';
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/observability_solution/apm/common/waterfall/typings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { TimestampUs } from '../../typings/es_schemas/raw/fields/timestamp_us';
import { AgentName } from '../../typings/es_schemas/ui/fields/agent';

export interface WaterfallTransaction {
'@timestamp': string;
timestamp: TimestampUs;
trace: { id: string };
service: {
Expand All @@ -38,6 +39,7 @@ export interface WaterfallTransaction {
}

export interface WaterfallSpan {
'@timestamp': string;
timestamp: TimestampUs;
trace: { id: string };
service: {
Expand Down Expand Up @@ -70,6 +72,7 @@ export interface WaterfallSpan {
}

export interface WaterfallError {
'@timestamp': string;
timestamp: TimestampUs;
trace?: { id: string };
transaction?: { id: string };
Expand Down
5 changes: 4 additions & 1 deletion ...ution/apm/public/components/app/error_group_details/error_sampler/error_sample_detail.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { first } from 'lodash';
import React, { useEffect, useState } from 'react';
import { useHistory } from 'react-router-dom';
import useAsync from 'react-use/lib/useAsync';
import { getTimestamp } from '../../../../utils/get_timestamp';
import { ERROR_GROUP_ID } from '../../../../../common/es_fields/apm';
import { TraceSearchType } from '../../../../../common/trace_explorer';
import { APMError } from '../../../../../typings/es_schemas/ui/apm_error';
Expand Down Expand Up @@ -175,6 +176,8 @@ export function ErrorSampleDetails({
},
});

const timestamp = getTimestamp(errorData?.error?.['@timestamp'], errorData?.error?.timestamp?.us);

return (
<EuiPanel hasBorder={true}>
<EuiFlexGroup alignItems="center">
Expand Down Expand Up @@ -245,7 +248,7 @@ export function ErrorSampleDetails({
) : (
<Summary
items={[
<TimestampTooltip time={errorData ? error.timestamp.us / 1000 : 0} />,
<TimestampTooltip time={errorData ? timestamp / 1000 : 0} />,
errorUrl && method ? (
<HttpInfoSummaryItem url={errorUrl} method={method} status={status} />
) : null,
Expand Down
5 changes: 4 additions & 1 deletion ...apm/public/components/app/transaction_details/waterfall_with_summary/transaction_tabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { EuiSpacer, EuiTab, EuiTabs, EuiSkeletonText } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { LogStream } from '@kbn/logs-shared-plugin/public';
import React, { useMemo } from 'react';
import { getTimestamp } from '../../../../utils/get_timestamp';
import { Transaction } from '../../../../../typings/es_schemas/ui/transaction';
import { TransactionMetadata } from '../../../shared/metadata_table/transaction_metadata';
import { WaterfallContainer } from './waterfall_container';
Expand Down Expand Up @@ -43,6 +44,7 @@ export function TransactionTabs({
showCriticalPath,
onShowCriticalPathChange,
}: Props) {
const timestamp = getTimestamp(transaction?.['@timestamp'], transaction?.timestamp?.us);
const tabs: Record<TransactionTab, { label: string; component: React.ReactNode }> = useMemo(
() => ({
[TransactionTab.timeline]: {
Expand Down Expand Up @@ -73,7 +75,7 @@ export function TransactionTabs({
<>
{transaction && (
<LogsTabContent
timestamp={transaction.timestamp.us}
timestamp={timestamp}
duration={transaction.transaction.duration.us}
traceId={transaction.trace.id}
/>
Expand All @@ -89,6 +91,7 @@ export function TransactionTabs({
transaction,
waterfall,
waterfallItemId,
timestamp,
]
);

Expand Down
5 changes: 4 additions & 1 deletion ...action_details/waterfall_with_summary/waterfall_container/waterfall/span_flyout/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { euiStyled } from '@kbn/kibana-react-plugin/common';
import { ProcessorEvent } from '@kbn/observability-plugin/common';
import { isEmpty } from 'lodash';
import React, { Fragment } from 'react';
import { getTimestamp } from '../../../../../../../utils/get_timestamp';
import { PlaintextStacktrace } from '../../../../../error_group_details/error_sampler/plaintext_stacktrace';
import { Span } from '../../../../../../../../typings/es_schemas/ui/span';
import { Transaction } from '../../../../../../../../typings/es_schemas/ui/transaction';
Expand Down Expand Up @@ -261,13 +262,15 @@ function SpanFlyoutBody({
];

const initialTab = tabs.find(({ id }) => id === flyoutDetailTab) ?? tabs[0];
const timestamp = getTimestamp(span['@timestamp'], span.timestamp.us);

return (
<>
<StickySpanProperties span={span} transaction={parentTransaction} />
<EuiSpacer size="m" />
<Summary
items={[
<TimestampTooltip time={span.timestamp.us / 1000} />,
<TimestampTooltip time={timestamp / 1000} />,
<>
<DurationSummaryItem
duration={span.span.duration.us}
Expand Down
36 changes: 29 additions & 7 deletions ...terfall_with_summary/waterfall_container/waterfall/waterfall_helpers/waterfall_helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
WaterfallSpan,
WaterfallTransaction,
} from '../../../../../../../../common/waterfall/typings';
import { getTimestamp } from '../../../../../../../utils/get_timestamp';

type TraceAPIResponse = APIReturnType<'GET /internal/apm/traces/{traceId}'>;

Expand Down Expand Up @@ -166,18 +167,23 @@ function getErrorItem(
items: IWaterfallItem[],
entryWaterfallTransaction?: IWaterfallTransaction
): IWaterfallError {
const entryTimestamp = entryWaterfallTransaction?.doc.timestamp.us ?? 0;
const timestamp = getTimestamp(
entryWaterfallTransaction?.doc['@timestamp'],
entryWaterfallTransaction?.doc.timestamp.us
);
const entryTimestamp = timestamp ?? 0;
const parent = items.find((waterfallItem) => waterfallItem.id === error.parent?.id) as
| IWaterfallSpanOrTransaction
| undefined;

const errorTimestamp = getTimestamp(error['@timestamp'], error.timestamp.us);
const errorItem: IWaterfallError = {
docType: 'error',
doc: error,
id: error.error.id,
parent,
parentId: parent?.id,
offset: error.timestamp.us - entryTimestamp,
offset: errorTimestamp - entryTimestamp,
skew: 0,
color: '',
};
Expand All @@ -202,10 +208,15 @@ export function getClockSkew(
return parentItem.skew;
// transaction is the initial entry in a service. Calculate skew for this, and it will be propagated to all child spans
case 'transaction': {
const parentStart = parentItem.doc.timestamp.us + parentItem.skew;
const parentTimestamp = getTimestamp(
parentItem.doc['@timestamp'],
parentItem.doc.timestamp.us
);
const parentStart = parentTimestamp + parentItem.skew;

// determine if child starts before the parent
const offsetStart = parentStart - item.doc.timestamp.us;
const itemTimestamp = getTimestamp(item.doc['@timestamp'], item.doc.timestamp.us);
const offsetStart = parentStart - itemTimestamp;
if (offsetStart > 0) {
const latency = Math.max(parentItem.duration - item.duration, 0) / 2;
return offsetStart + latency;
Expand All @@ -224,7 +235,11 @@ export function getOrderedWaterfallItems(
if (!entryWaterfallTransaction) {
return [];
}
const entryTimestamp = entryWaterfallTransaction.doc.timestamp.us;
const timestamp = getTimestamp(
entryWaterfallTransaction.doc['@timestamp'],
entryWaterfallTransaction.doc.timestamp.us
);
const entryTimestamp = timestamp;
const visitedWaterfallItemSet = new Set();

function getSortedChildren(
Expand All @@ -236,11 +251,18 @@ export function getOrderedWaterfallItems(
}
visitedWaterfallItemSet.add(item);

const children = sortBy(childrenByParentId[item.id] || [], 'doc.timestamp.us');
const children = sortBy(childrenByParentId[item.id] || [], (child) => {
if (!child.doc.timestamp.us && child.doc['@timestamp']) {
return `doc['@timestamp']`;
}

return 'doc.timestamp.us';
});

item.parent = parentItem;
// get offset from the beginning of trace
item.offset = item.doc.timestamp.us - entryTimestamp;
const itemTimestamp = getTimestamp(item.doc['@timestamp'], item.doc.timestamp.us);
item.offset = itemTimestamp - entryTimestamp;
// move the item to the right if it starts before its parent
item.skew = getClockSkew(item, parentItem);

Expand Down
4 changes: 3 additions & 1 deletion ...ugins/observability_solution/apm/public/components/shared/summary/transaction_summary.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import React from 'react';
import { getTimestamp } from '../../../utils/get_timestamp';
import { Transaction } from '../../../../typings/es_schemas/ui/transaction';
import { Summary } from '.';
import { TimestampTooltip } from '../timestamp_tooltip';
Expand Down Expand Up @@ -42,8 +43,9 @@ function getTransactionResultSummaryItem(transaction: Transaction) {
}

function TransactionSummary({ transaction, totalDuration, errorCount, coldStartBadge }: Props) {
const timestamp = getTimestamp(transaction['@timestamp'], transaction.timestamp.us);
const items = [
<TimestampTooltip time={transaction.timestamp.us / 1000} />,
<TimestampTooltip time={timestamp / 1000} />,
<DurationSummaryItem
duration={transaction.transaction.duration.us}
totalDuration={totalDuration}
Expand Down
5 changes: 3 additions & 2 deletions ...s/observability_solution/apm/public/components/shared/transaction_action_menu/sections.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type { ProfilingLocators } from '@kbn/observability-shared-plugin/public'
import type { AssetDetailsLocator } from '@kbn/observability-shared-plugin/common';
import { LocatorPublic } from '@kbn/share-plugin/common';
import { SerializableRecord } from '@kbn/utility-types';
import { getTimestamp } from '../../../utils/get_timestamp';
import { Environment } from '../../../../common/environment_rt';
import type { Transaction } from '../../../../typings/es_schemas/ui/transaction';
import { getDiscoverHref } from '../links/discover_links/discover_link';
Expand Down Expand Up @@ -68,8 +69,8 @@ export const getSections = ({
const hostName = transaction.host?.hostname;
const podId = transaction.kubernetes?.pod?.uid;
const containerId = transaction.container?.id;

const time = Math.round(transaction.timestamp.us / 1000);
const timestamp = getTimestamp(transaction['@timestamp'], transaction.timestamp.us);
const time = Math.round(timestamp / 1000);
const infraMetricsQuery = getInfraMetricsQuery(transaction);

const uptimeLink = uptimeLocator?.getRedirectUrl(
Expand Down
14 changes: 14 additions & 0 deletions x-pack/plugins/observability_solution/apm/public/utils/get_timestamp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export function getTimestamp(timestamp?: string, fallbackTimestamp?: number): number {
if (!fallbackTimestamp && timestamp) {
Copy link
Contributor

Choose a reason for hiding this comment

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

In case timestamp and fallbackTimestamp are both true then fallbackTimestamp is returned. I think it would be better to rely on timestamp in that scenario, wdyt?

maybe an opposite condition?

if (!timestamp && fallbackTimestamp) {
  return  fallbackTimestamp;
}

return new Date(timestamp).getTime() * 1000 || 0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would make sense, let's see what others think and I can implement it if everyone agrees

Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't have timestamp.us, we return @timestamp ✅

Can we rename these variables? Because as per this comment ☝️ I was confused of which one is the fallback @timestamp or timestamp_us

const timestamp = getTimestamp(errorData?.error?.['@timestamp'], errorData?.error?.timestamp?.us);

In this case fallback is timestamp_us, but if we don't have it then the 'fallback' is @timestamp

I will keep the variables with the same name or close to the real field timestamp & timestampUs to avoid confusion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to timestamp and timestampUs as the order of the params should always be the same

return new Date(timestamp).getTime() * 1000;
Copy link
Member

Choose a reason for hiding this comment

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

This will truncate the microsecond precision when the timestamp is of type date_nanos. Is there a way to retain the microsecond precision?

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 did a refactor to keep the microsecond precision

}

return fallbackTimestamp || 0;
}
3 changes: 3 additions & 0 deletions x-pack/plugins/observability_solution/apm/server/routes/traces/get_trace_items.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
ERROR_LOG_MESSAGE,
EVENT_OUTCOME,
FAAS_COLDSTART,
FALLBACK_TIMESTAMP,
Copy link
Contributor

Choose a reason for hiding this comment

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

Same in here
TIMESTAMP & TIMESTAMP_US, it's more clear IMHO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

PARENT_ID,
PROCESSOR_EVENT,
SERVICE_ENVIRONMENT,
Expand Down Expand Up @@ -98,6 +99,7 @@ export async function getTraceItems({
size: 1000,
_source: [
TIMESTAMP,
FALLBACK_TIMESTAMP,
TRACE_ID,
TRANSACTION_ID,
PARENT_ID,
Expand Down Expand Up @@ -229,6 +231,7 @@ async function getTraceDocsPerPage({
search_after: searchAfter,
_source: [
TIMESTAMP,
FALLBACK_TIMESTAMP,
TRACE_ID,
PARENT_ID,
SERVICE_NAME,
Expand Down
Loading