-
Notifications
You must be signed in to change notification settings - Fork 0
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
Otel collector mappings to APM agent #6
Changes from 18 commits
03a86d9
2e97e81
a6244f5
cb9f1d4
0b86fc2
f54e2b3
37dd831
8e55406
0cabafb
f5e627c
c52ddfd
e77d9a4
f8f9b78
3eac633
d467a38
ac07c73
8666c00
06545b5
a08ef93
f156559
2ef8527
004f587
354d821
aebea67
3f99bae
b4c5a95
9d040cf
e4fac40
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 |
---|---|---|
|
@@ -173,6 +173,7 @@ export async function getServiceMetadataDetails({ | |
const response = await apmEventClient.search('get_service_metadata_details', params); | ||
|
||
const fields = response.hits.hits[0]?.fields; | ||
// todo: missing `fields` property? | ||
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. ? 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 haven't found I'm not sure whether we shouldn't use ![]() 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 see. I'll take a closer look. It's unclear if it's only using the aggregations and fields are not required. 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. The issue looks like the
which are the values:
We should either use |
||
const fieldsNorm = (fields ? normalizeFields(fields) : undefined) as | ||
| ServiceMetadataDetailsRaw | ||
| undefined; | ||
|
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.
If this returns APMError, special care will need to be taken. I'm not sure that
fields
is capable of returning fields fromerror.exception.stacktrace
. We may have to depend on _source in this situation.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.
Here's an example query for demonstration.
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.
I don't know if that's even a problem since we
Pick
onlyobserver
property from the type when doing assertion. And, this is the only usage as far I can see: https://github.com/jennypavlova/kibana/pull/6/files/f52184082c369d7f63e03b250d0a09c212644f48#diff-8c8160096093f32c6f9dd260780403b55fe20f94f2c99de76cc9d54c872cb4e8R701I'm not entirely sure why we assert type to
Transaction | Span | APMError
. I couldn't even find a way to trigger this code on my local environment. That's why I lefttodo
comment to double-check it later, but I think I'm out of ideas here. Do you have any suggestion?