-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
🚧 WIP 🚧 Replace _source in the APM queries #192608
Changes from 70 commits
45ff639
b082ebd
e3ca03e
91a5bbc
73a5731
0354d3a
8d921bb
8e5614c
a9474cd
a246c31
53dabf7
55ea137
638b1cc
00d88b5
02ef2f5
bb8c710
536773d
32c2a59
2a67dc9
03a86d9
2e97e81
a6244f5
cb9f1d4
0b86fc2
f54e2b3
37dd831
8e55406
0cabafb
f5e627c
c52ddfd
e77d9a4
f8f9b78
3eac633
d467a38
ac07c73
cef6b68
2e26dc5
8666c00
8a66ac7
5dbdbc4
06545b5
a08ef93
f156559
e7da158
45f6285
0c6a4cb
4372e43
2ef8527
004f587
354d821
aebea67
60e4870
fe9be17
3f99bae
b4c5a95
9d040cf
e4fac40
634b899
9438e85
850b331
12ad0a5
9bcb917
12dffee
8bcec36
05acbbb
51b0f0b
4d866bb
f9b8b23
ba589cd
e8362c3
65a3580
d11b7e4
0a98a64
c7eeaa0
3dd2acb
e5b99df
cc592fb
d069d97
fe06eb5
cf86079
d4b4a83
8a27f0d
95f2cf7
7a34150
d7d4372
f083497
ec61907
7142a51
1a99043
630c6d0
57338e1
d543e39
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -160,7 +160,7 @@ export function ErrorSampleDetails({ | |
const status = error.http?.response?.status_code; | ||
const environment = error.service.environment; | ||
const serviceVersion = error.service.version; | ||
const isUnhandled = error.error.exception?.[0].handled === false; | ||
const isUnhandled = error.error.exception?.[0]?.handled === false; | ||
|
||
const traceExplorerLink = router.link('/traces/explorer/waterfall', { | ||
query: { | ||
|
@@ -352,8 +352,8 @@ export function ErrorSampleDetailTabContent({ | |
currentTab: ErrorTab; | ||
}) { | ||
const codeLanguage = error?.service.language?.name; | ||
const exceptions = error?.error.exception || []; | ||
const logStackframes = error?.error.log?.stacktrace; | ||
const exceptions = error?.error?.exception || []; | ||
const logStackframes = error?.error?.log?.stacktrace; | ||
const isPlaintextException = | ||
!!error?.error.stack_trace && exceptions.length === 1 && !exceptions[0].stacktrace; | ||
switch (currentTab.key) { | ||
|
@@ -362,8 +362,8 @@ export function ErrorSampleDetailTabContent({ | |
case ErrorTabKey.ExceptionStacktrace: | ||
return isPlaintextException ? ( | ||
<PlaintextStacktrace | ||
message={exceptions[0].message} | ||
type={exceptions[0].type} | ||
message={exceptions[0]?.message} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should not be necessary either, given the assertion on line 357 ( |
||
type={exceptions[0]?.type} | ||
stacktrace={error?.error.stack_trace} | ||
codeLanguage={codeLanguage} | ||
/> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -174,7 +174,8 @@ function getErrorItem( | |
const errorItem: IWaterfallError = { | ||
docType: 'error', | ||
doc: error, | ||
id: error.error.id, | ||
// To be fixed after https://github.com/elastic/opentelemetry-lib/issues/86 | ||
id: error?.error?.id ?? '', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are these needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because of the blocker here - the error id in the Otel PoC data is missing currently. I will update it once it is fixed and I added it to unblock the testing of the other parts There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I linked the issue in a comment |
||
parent, | ||
parentId: parent?.id, | ||
offset: error.timestamp.us - entryTimestamp, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -193,7 +193,7 @@ function NameLabel({ item }: { item: IWaterfallSpanOrTransaction }) { | |
switch (item.docType) { | ||
case 'span': | ||
let name = item.doc.span.name; | ||
if (item.doc.span.composite) { | ||
if (item.doc.span.composite && item.doc.span.composite.count) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does it matter that this returns false when count is 0? probably not, but might be good to be more specific here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It was shown in the name so I had |
||
const compositePrefix = | ||
item.doc.span.composite.compression_strategy === 'exact_match' ? 'x' : ''; | ||
name = `${item.doc.span.composite.count}${compositePrefix} ${name}`; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,6 @@ export async function getDownstreamServiceResource({ | |
body: { | ||
track_total_hits: false, | ||
size: 1, | ||
_source: ['span.destination.service'], | ||
query: { | ||
bool: { | ||
filter: [ | ||
|
@@ -50,9 +49,10 @@ export async function getDownstreamServiceResource({ | |
], | ||
}, | ||
}, | ||
fields: ['span.destination.service'], | ||
jennypavlova marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
}); | ||
|
||
const hit = response.hits.hits[0]; | ||
return hit?._source?.span.destination?.service.resource; | ||
return hit?.fields?.['span.destination?.service?.resource']?.[0] as string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question marks here in the string need to be removed and the type should be |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ import { environmentQuery } from '../../../common/utils/environment_query'; | |
import { EntitiesESClient } from '../../lib/helpers/create_es_client/create_entities_es_client/create_entities_es_client'; | ||
import { entitiesRangeQuery } from './get_entities'; | ||
import { EntityLatestServiceRaw, EntityType } from './types'; | ||
import { normalizeFields } from '../../utils/normalize_fields'; | ||
|
||
export async function getEntityLatestServices({ | ||
entitiesESClient, | ||
|
@@ -40,7 +41,7 @@ export async function getEntityLatestServices({ | |
body: { | ||
size, | ||
track_total_hits: false, | ||
_source: [AGENT_NAME, ENTITY, DATA_STEAM_TYPE, SERVICE_NAME, SERVICE_ENVIRONMENT], | ||
fields: [AGENT_NAME, ENTITY, DATA_STEAM_TYPE, SERVICE_NAME, SERVICE_ENVIRONMENT], | ||
query: { | ||
bool: { | ||
filter: [ | ||
|
@@ -54,7 +55,7 @@ export async function getEntityLatestServices({ | |
}, | ||
}, | ||
}) | ||
).hits.hits.map((hit) => hit._source); | ||
).hits.hits.map((hit) => normalizeFields(hit?.fields ?? {}) as unknown as EntityLatestServiceRaw); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. need to replace with an explicit mapping, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! It's on our list to replace this function everywhere. We discussed that with @miloszmarcinkowski and I am wondering if we want to have this change for the services that are coming from EEM - so the view is still in tech preview and idk if we want to have the changes there as this should not affect open telemetry "classic" view. Do we want to do this change there as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes please. In fact, any new feature being developed in the UI should be from now on "OTel-native" (i.e. not using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I reverted the changes and it won't affect the services with Otel ( I see no errors when I enabled the new view ) |
||
|
||
return latestEntityServices; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the first
?
is not necessary -APMError
is always defined (I understand these were here before, but let's not add to it)