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

[Backport 2.x] Traces jaeger #214

Merged
merged 1 commit into from
Jan 24, 2023
Merged

[Backport 2.x] Traces jaeger #214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions common/constants/explorer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const REDIRECT_TAB = 'redirect_tab';
export const PAGE_SIZE = 50;
export const DEFAULT_COLUMNS = ['', 'Time', '_source'];
export const OTEL_TRACE_ID = 'traceId';
export const JAEGER_TRACE_ID = 'traceID';
export const DATE_PICKER_FORMAT = 'YYYY-MM-DD HH:mm:ss';
export const TIME_INTERVAL_OPTIONS = [
{
Expand Down
5 changes: 4 additions & 1 deletion common/constants/trace_analytics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
* SPDX-License-Identifier: Apache-2.0
*/

export const JAEGER_INDEX_NAME = '*jaeger-span-*';
export const JAEGER_SERVICE_INDEX_NAME = '*jaeger-service*';
export const DATA_PREPPER_INDEX_NAME = 'otel-v1-apm-span-*';
export const DATA_PREPPER_SERVICE_INDEX_NAME = 'otel-v1-apm-service-map*';
export const TRACE_ANALYTICS_DATE_FORMAT = 'MM/DD/YYYY HH:mm:ss';
Expand All @@ -13,5 +15,6 @@ export const SERVICE_MAP_MAX_EDGES = 1000;
export const TRACES_MAX_NUM = 3000;
export const TRACE_ANALYTICS_DOCUMENTATION_LINK = 'https://opensearch.org/docs/latest/observability-plugin/trace/index/';

export const TRACE_ANALYTICS_INDICES_ROUTE = '/api/observability/trace_analytics/indices';
export const TRACE_ANALYTICS_JAEGER_INDICES_ROUTE = '/api/observability/trace_analytics/jaeger_indices';
export const TRACE_ANALYTICS_DATA_PREPPER_INDICES_ROUTE = '/api/observability/trace_analytics/data_prepper_indices';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: do we want to add an endpoint base variable? something like

const TRACE_ENDPOINT_BASE = '/api/observability/trace_analytics';
export const TRACE_ANALYTICS_JAEGER_INDICES_ROUTE = `${TRACE_ENDPOINT_BASE}/jaeger_indices`;
export const TRACE_ANALYTICS_DATA_PREPPER_INDICES_ROUTE = `${TRACE_ENDPOINT_BASE}/data_prepper_indices`;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good idea - will do

export const TRACE_ANALYTICS_DSL_ROUTE = '/api/observability/trace_analytics/query';
4 changes: 2 additions & 2 deletions public/components/application_analytics/__tests__/__snapshots__/service_config.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ exports[`Service Config component renders empty service config 1`] = `
endTime="now"
filters={Array []}
http={[MockFunction]}
indicesExist={true}
mode="data_prepper"
name=""
parentBreadcrumb={
Object {
Expand Down Expand Up @@ -1101,7 +1101,7 @@ exports[`Service Config component renders with one service selected 1`] = `
]
}
http={[MockFunction]}
indicesExist={true}
mode="data_prepper"
name=""
parentBreadcrumb={
Object {
Expand Down
16 changes: 8 additions & 8 deletions public/components/application_analytics/__tests__/create.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ describe('Create Page', () => {
setStartTime={setStartTime}
endTime="now"
setEndTime={setEndTime}
indicesExist={true}
name=""
description=""
mode='data_prepper'
setNameWithStorage={setNameWithStorage}
setDescriptionWithStorage={setDescriptionWithStorage}
setQueryWithStorage={setQueryWithStorage}
Expand Down Expand Up @@ -120,9 +120,9 @@ describe('Create Page', () => {
setStartTime={setStartTime}
endTime="now"
setEndTime={setEndTime}
indicesExist={true}
name="Chic Application"
description="This is my chic application."
mode='data_prepper'
setNameWithStorage={setNameWithStorage}
setDescriptionWithStorage={setDescriptionWithStorage}
setQueryWithStorage={setQueryWithStorage}
Expand Down Expand Up @@ -184,9 +184,9 @@ describe('Create Page', () => {
setStartTime={setStartTime}
endTime="now"
setEndTime={setEndTime}
indicesExist={true}
name=""
description=""
mode='data_prepper'
setNameWithStorage={setNameWithStorage}
setDescriptionWithStorage={setDescriptionWithStorage}
setQueryWithStorage={setQueryWithStorage}
Expand Down Expand Up @@ -251,9 +251,9 @@ describe('Create Page', () => {
setStartTime={setStartTime}
endTime="now"
setEndTime={setEndTime}
indicesExist={true}
name=""
description=""
mode='data_prepper'
setNameWithStorage={setNameWithStorage}
setDescriptionWithStorage={setDescriptionWithStorage}
setQueryWithStorage={setQueryWithStorage}
Expand Down Expand Up @@ -327,9 +327,9 @@ describe('Create Page', () => {
setStartTime={setStartTime}
endTime="now"
setEndTime={setEndTime}
indicesExist={true}
name=""
description=""
mode='data_prepper'
setNameWithStorage={setNameWithStorage}
setDescriptionWithStorage={setDescriptionWithStorage}
setQueryWithStorage={setQueryWithStorage}
Expand Down Expand Up @@ -403,9 +403,9 @@ describe('Create Page', () => {
setStartTime={setStartTime}
endTime="now"
setEndTime={setEndTime}
indicesExist={true}
name=""
description=""
mode='data_prepper'
setNameWithStorage={setNameWithStorage}
setDescriptionWithStorage={setDescriptionWithStorage}
setQueryWithStorage={setQueryWithStorage}
Expand Down Expand Up @@ -479,9 +479,9 @@ describe('Create Page', () => {
setStartTime={setStartTime}
endTime="now"
setEndTime={setEndTime}
indicesExist={true}
name=""
description=""
mode='data_prepper'
setNameWithStorage={setNameWithStorage}
setDescriptionWithStorage={setDescriptionWithStorage}
setQueryWithStorage={setQueryWithStorage}
Expand Down Expand Up @@ -555,9 +555,9 @@ describe('Create Page', () => {
setStartTime={setStartTime}
endTime="now"
setEndTime={setEndTime}
indicesExist={true}
name=""
description=""
mode='data_prepper'
setNameWithStorage={setNameWithStorage}
setDescriptionWithStorage={setDescriptionWithStorage}
setQueryWithStorage={setQueryWithStorage}
Expand Down
4 changes: 2 additions & 2 deletions public/components/application_analytics/__tests__/service_config.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('Service Config component', () => {
setStartTime={setStartTime}
endTime="now"
setEndTime={setEndTime}
indicesExist={true}
mode='data_prepper'
dslService={dslService}
selectedServices={[]}
setSelectedServices={setSelectedServices}
Expand Down Expand Up @@ -108,7 +108,7 @@ describe('Service Config component', () => {
setStartTime={setStartTime}
endTime="now"
setEndTime={setEndTime}
indicesExist={true}
mode='data_prepper'
dslService={dslService}
selectedServices={[]}
setSelectedServices={setSelectedServices}
Expand Down
5 changes: 4 additions & 1 deletion public/components/application_analytics/components/application.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export function Application(props: AppDetailProps) {
setFilters,
callback,
queryManager,
mode,
} = props;
const [application, setApplication] = useState<ApplicationType>({
id: '',
Expand Down Expand Up @@ -230,7 +231,7 @@ export function Application(props: AppDetailProps) {
}, [appId, application.name]);

useEffect(() => {
const DSL = filtersToDsl(filters, query, appStartTime, appEndTime, 'app', appConfigs);
const DSL = filtersToDsl(mode, filters, query, appStartTime, appEndTime, 'app', appConfigs);
setSpanDSL(DSL);
}, [filters, appConfigs, query, appStartTime, appEndTime]);

Expand Down Expand Up @@ -350,6 +351,7 @@ export function Application(props: AppDetailProps) {
openFlyout={setSpanFlyoutId}
DSL={spanDSL}
setTotal={setTotalSpans}
mode='data_prepper'
/>
</EuiPanel>
</>
Expand Down Expand Up @@ -563,6 +565,7 @@ export function Application(props: AppDetailProps) {
isFlyoutVisible={!!spanFlyoutId}
closeFlyout={closeSpanFlyout}
addSpanFilter={addSpanFilter}
mode="data_prepper"
/>
)}
{traceFlyoutId && (
Expand Down
3 changes: 2 additions & 1 deletion public/components/application_analytics/components/config_components/service_config.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const ServiceConfig = (props: ServiceConfigProps) => {
selectedServices,
setSelectedServices,
} = props;
const { mode } = props;
const [servicesOpen, setServicesOpen] = useState(false);
const [serviceMap, setServiceMap] = useState<ServiceObject>({});
const [serviceMapIdSelected, setServiceMapIdSelected] = useState<
Expand All @@ -49,7 +50,7 @@ export const ServiceConfig = (props: ServiceConfigProps) => {
const [modalLayout, setModalLayout] = useState(<EuiOverlayMask />);

useEffect(() => {
handleServiceMapRequest(http, dslService, setServiceMap);
handleServiceMapRequest(http, dslService, mode, setServiceMap);
}, []);

useEffect(() => {
Expand Down
8 changes: 5 additions & 3 deletions public/components/application_analytics/components/config_components/trace_config.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
} from '@elastic/eui';
import DSLService from 'public/services/requests/dsl';
import React, { useEffect, useState } from 'react';
import { FilterType } from 'public/components/trace_analytics/components/common/filters/filters';
import { FilterType } from '../../../../../public/components/trace_analytics/components/common/filters/filters';
import { OptionType } from '../../../../../common/types/application_analytics';
import { filtersToDsl } from '../../../trace_analytics/components/common/helper_functions';
import { handleDashboardRequest } from '../../../trace_analytics/requests/dashboard_request_handler';
Expand All @@ -42,6 +42,7 @@ export const TraceConfig = (props: TraceConfigProps) => {
endTime,
selectedTraces,
setSelectedTraces,
mode
} = props;
const [traceOpen, setTraceOpen] = useState(false);
const [loading, setLoading] = useState(false);
Expand All @@ -54,16 +55,17 @@ export const TraceConfig = (props: TraceConfigProps) => {

useEffect(() => {
setLoading(true);
const timeFilterDSL = filtersToDsl([], '', startTime, endTime);
const timeFilterDSL = filtersToDsl(mode, [], '', startTime, endTime);
const latencyTrendStartTime = dateMath.parse(endTime, { roundUp: true })?.subtract(24, 'hours').toISOString()!;
const latencyTrendDSL = filtersToDsl(filters, query, latencyTrendStartTime, endTime);
const latencyTrendDSL = filtersToDsl(mode, filters, query, latencyTrendStartTime, endTime);
handleDashboardRequest(
http,
dslService,
timeFilterDSL,
latencyTrendDSL,
traceItems,
setTraceItems,
mode,
setPercentileMap
).then(() => setLoading(false));
setRedirect(false);
Expand Down
12 changes: 7 additions & 5 deletions ...c/components/application_analytics/components/flyout_components/service_detail_flyout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
handleServiceMapRequest,
handleServiceViewRequest,
} from '../../../../../public/components/trace_analytics/requests/services_request_handler';
import { filtersToDsl } from '../../../../../public/components/trace_analytics/components/common/helper_functions';
import { filtersToDsl, processTimeStamp } from '../../../../../public/components/trace_analytics/components/common/helper_functions';
import { ServiceMap } from '../../../../../public/components/trace_analytics/components/services';
import { ServiceObject } from '../../../../../public/components/trace_analytics/components/common/plots/service_map';
import { SpanDetailTable } from '../../../../../public/components/trace_analytics/components/traces/span_detail_table';
Expand All @@ -44,6 +44,7 @@ export function ServiceDetailFlyout(props: ServiceFlyoutProps) {
query,
closeServiceFlyout,
openSpanFlyout,
mode,
} = props;
const [fields, setFields] = useState<any>({});
const [serviceMap, setServiceMap] = useState<ServiceObject>({});
Expand Down Expand Up @@ -110,16 +111,17 @@ export function ServiceDetailFlyout(props: ServiceFlyoutProps) {
DSL={DSL}
openFlyout={openSpanFlyout}
setTotal={setTotal}
mode={mode}
/>
</>
);
}, [serviceName, fields, serviceMap, DSL, serviceMapIdSelected]);

useEffect(() => {
const serviceDSL = filtersToDsl(filters, query, startTime, endTime, 'app', appConfigs);
handleServiceViewRequest(serviceName, http, serviceDSL, setFields);
handleServiceMapRequest(http, serviceDSL, setServiceMap, serviceName);
const spanDSL = filtersToDsl(filters, query, startTime, endTime, 'app', appConfigs);
const serviceDSL = filtersToDsl(mode, filters, query, processTimeStamp(startTime, mode), processTimeStamp(endTime, mode), 'app', appConfigs);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel this and some other functions have too many params passed in as separate params, what we could do here is to maybe change the function definition to something like filtersToDsl(dslData) or filtersToDsl({mode, filters, ...}) and here you can pass them as one obj

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just kept it consistent to not change too much with one PR - wondering for my own curiosity where you are coming from - is this mainly for readability/line length? What's the benefit of passing in an object with the same params vs. just passing in the params by themselves?

Copy link
Collaborator

Choose a reason for hiding this comment

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

There are references out there explaining why to not have a long parameter list but in general it helps for not only better readability/line length, but also easier scaling, and also sometimes a long param list is an indicator of not having single responsibility for a function. But sure, not need to rush into one PR.

handleServiceViewRequest(serviceName, http, serviceDSL, setFields, mode);
handleServiceMapRequest(http, serviceDSL, mode, setServiceMap, serviceName);
const spanDSL = filtersToDsl(mode, filters, query, startTime, endTime, 'app', appConfigs);
spanDSL.query.bool.must.push({
term: {
serviceName,
Expand Down
2 changes: 1 addition & 1 deletion public/components/application_analytics/components/flyout_components/trace_detail_flyout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ interface TraceFlyoutProps extends TraceAnalyticsComponentDeps {
export function TraceDetailFlyout(props: TraceFlyoutProps) {
const { traceId, http, closeTraceFlyout, openSpanFlyout } = props;
const renderContent = (
<TraceDetailRender traceId={traceId} http={http} openSpanFlyout={openSpanFlyout} />
<TraceDetailRender traceId={traceId} http={http} openSpanFlyout={openSpanFlyout} mode='data_prepper'/>
);
return (
<EuiFlyout data-test-subj="traceDetailFlyout" onClose={closeTraceFlyout} size="m">
Expand Down
11 changes: 7 additions & 4 deletions public/components/application_analytics/components/flyout_components/trace_detail_render.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@ import {
} from '../../../trace_analytics/requests/traces_request_handler';
import { HttpStart } from '../../../../../../../src/core/public';
import { getListItem } from '../../helpers/utils';
import { TraceAnalyticsMode } from '../../../../../public/components/trace_analytics/home';

interface TraceDetailRenderProps {
traceId: string;
http: HttpStart;
openSpanFlyout: (spanId: string) => void;
mode : TraceAnalyticsMode
}

export const TraceDetailRender = ({ traceId, http, openSpanFlyout }: TraceDetailRenderProps) => {
export const TraceDetailRender = ({ traceId, http, openSpanFlyout, mode }: TraceDetailRenderProps) => {
const [fields, setFields] = useState<any>({});
const [serviceBreakdownData, setServiceBreakdownData] = useState([]);
const [payloadData, setPayloadData] = useState('');
Expand Down Expand Up @@ -66,6 +68,7 @@ export const TraceDetailRender = ({ traceId, http, openSpanFlyout }: TraceDetail
colorMap={colorMap}
page="app"
openSpanFlyout={openSpanFlyout}
mode={mode}
/>
<EuiSpacer size="xs" />
<EuiHorizontalRule margin="s" />
Expand All @@ -83,9 +86,9 @@ export const TraceDetailRender = ({ traceId, http, openSpanFlyout }: TraceDetail
}, [traceId, fields, serviceBreakdownData, colorMap, payloadData]);

useEffect(() => {
handleTraceViewRequest(traceId, http, fields, setFields);
handleServicesPieChartRequest(traceId, http, setServiceBreakdownData, setColorMap);
handlePayloadRequest(traceId, http, payloadData, setPayloadData);
handleTraceViewRequest(traceId, http, fields, setFields, mode);
handleServicesPieChartRequest(traceId, http, setServiceBreakdownData, setColorMap, mode);
handlePayloadRequest(traceId, http, payloadData, setPayloadData, mode);
}, [traceId]);

return renderContent;
Expand Down
7 changes: 4 additions & 3 deletions public/components/application_analytics/home.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { Application } from './components/application';
import { CreateApp } from './components/create';
import { TraceAnalyticsComponentDeps, TraceAnalyticsCoreDeps } from '../trace_analytics/home';
import { FilterType } from '../trace_analytics/components/common/filters/filters';
import { handleIndicesExistRequest } from '../trace_analytics/requests/request_handler';
import { handleDataPrepperIndicesExistRequest } from '../trace_analytics/requests/request_handler';
import { ObservabilitySideBar } from '../common/side_nav';
import { NotificationsStart } from '../../../../../src/core/public';
import { APP_ANALYTICS_API_PREFIX } from '../../../common/constants/application_analytics';
Expand Down Expand Up @@ -114,7 +114,7 @@ export const Home = (props: HomeProps) => {
};

useEffect(() => {
handleIndicesExistRequest(http, setIndicesExist);
handleDataPrepperIndicesExistRequest(http, setIndicesExist);
}, []);

const commonProps: AppAnalyticsComponentDeps = {
Expand All @@ -137,7 +137,8 @@ export const Home = (props: HomeProps) => {
setStartTime,
endTime,
setEndTime,
indicesExist,
mode: 'data_prepper',
dataPrepperIndicesExist: indicesExist
};

const setToast = (title: string, color = 'success', text?: ReactChild) => {
Expand Down
6 changes: 3 additions & 3 deletions public/components/event_analytics/explorer/events_views/docViewRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { useEffect } from 'react';
import { IExplorerFields, IField } from '../../../../../common/types/explorer';
import { DocFlyout } from './doc_flyout';
import { HttpStart } from '../../../../../../../src/core/public';
import { OTEL_TRACE_ID, DATE_PICKER_FORMAT } from '../../../../../common/constants/explorer';
import { OTEL_TRACE_ID, DATE_PICKER_FORMAT, JAEGER_TRACE_ID } from '../../../../../common/constants/explorer';
import { SurroundingFlyout } from './surrounding_flyout';
import PPLService from '../../../../services/requests/ppl';
import { isValidTraceId } from '../../utils';
Expand Down Expand Up @@ -77,13 +77,13 @@ export const DocViewRow = forwardRef((props: IDocViewRowProps, ref) => {
<span>
<dl className="source truncate-by-height">
{toPairs(doc).map((entry: string[]) => {
const isTraceField = entry[0] === OTEL_TRACE_ID;
const isTraceField = (entry[0] === OTEL_TRACE_ID || entry[0] === JAEGER_TRACE_ID);
return (
<span key={uniqueId('grid-desc')}>
<dt>{entry[0]}:</dt>
<dd>
<span>
{isTraceField && isValidTraceId(entry[1]) && !isFlyout ? (
{isTraceField && (isValidTraceId(entry[1]) || entry[0] === JAEGER_TRACE_ID) && !isFlyout ? (
<EuiLink onClick={tracesFlyout}>{entry[1]}</EuiLink>
) : (
entry[1]
Expand Down
5 changes: 3 additions & 2 deletions public/components/event_analytics/explorer/events_views/trace_block/trace_block.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ interface props {
}

export const TraceBlock = ({ http, hit, logTraceId }: props) => {
if (logTraceId === '' || !isValidTraceId(logTraceId)) {
if ((!hit.traceID || hit.traceID.length === 0) && (logTraceId === '' || !isValidTraceId(logTraceId))){
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering if it would ever hit the second part hit.traceID.length === 0 as the type of traceID is string? And do we need to consider use cases like a space being handed correctly?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Regarding your first point, I think you're right - will fix. Regarding your second point I don't think so... shouldn't we assume abstraction between how data is stored? Otherwise we will be handling a lot of edge cases, as long as the UI doesn't break if its not stored in the way we expect we just don't provide the functionality.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if I fully understand but for second part, it seems to me that if traceID is a string like ' ', in theory it should be going into this branch to render 'No Trace Id found in the event' UI, but in current logic it would fail the (!hit.traceID || hit.traceID.length === 0) checking and continue to run code after it. It would cause the following code like const mode = (!hit.traceID || hit.traceID.length === 0) ? 'data_prepper' : 'jaeger' to run to get mode = 'jaeger' and then render TraceDetailRender with those data. I was just double checking if this case is something we are expecting, and would it cause potential bug.

return (
<>
<EuiCallOut iconType="help" title="No Trace Id found in the event.">
Expand All @@ -42,6 +42,7 @@ export const TraceBlock = ({ http, hit, logTraceId }: props) => {
</>
);
}
const mode = (!hit.traceID || hit.traceID.length === 0) ? 'data_prepper' : 'jaeger'

return <TraceDetailRender traceId={logTraceId} http={http} />;
return <TraceDetailRender traceId={hit.traceID || logTraceId} http={http} mode={mode}/>;
};
Loading