Otel collector mappings to APM agent #6

miloszmarcinkowski · 2024-09-18T11:57:14Z

Closes elastic#192749

Summary

This is part of an effort (elastic#192606) in replacing the usage of _source with fields while migrating APM to Otel data collector.

In this PR, we're adding mappers which expose only fields used in APM. This assures we stay type safe and we provide only necessary fields.

bryce-b · 2024-09-23T14:51:23Z

.../plugins/observability_solution/apm/server/lib/apm_telemetry/collect_data_telemetry/tasks.ts

-        Transaction | Span | APMError,
-        'observer'
-      >;
+      const hit = serviceVersionMapping(response.hits.hits[0]?.fields);


If this returns APMError, special care will need to be taken. I'm not sure that fields is capable of returning fields from error.exception.stacktrace. We may have to depend on _source in this situation.

Here's an example query for demonstration.

GET /_search { "fields": ["error.exception.message","error.exception.stacktrace.exclude_from_grouping"], "query": { "exists": { "field": "error.exception.message" } }, "size": 1 }

I don't know if that's even a problem since we Pick only observer 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-8c8160096093f32c6f9dd260780403b55fe20f94f2c99de76cc9d54c872cb4e8R701

I'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 left todo comment to double-check it later, but I think I'm out of ideas here. Do you have any suggestion?

bryce-b · 2024-09-23T14:55:49Z

...ck/plugins/observability_solution/apm/server/routes/services/get_service_metadata_details.ts

@@ -174,6 +174,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?


I haven't found fields property in get_service_metadata_details when running with otel-apm-e2e-poc

I'm not sure whether we shouldn't use fields property here or there isn't enough data in ES or there is some backend work needed here to get this field. What do you think @bryce-b?

I see. I'll take a closer look. It's unclear if it's only using the aggregations and fields are not required.

The issue looks like the fields selected in the query were copied from the _source values, which are root attributes.

fields: [SERVICE, AGENT, HOST, CONTAINER, KUBERNETES, CLOUD, LABEL_TELEMETRY_AUTO_VERSION],

which are the values:

"fields": [ "service", "agent", "host", "container", "kubernetes", "cloud", "labels.telemetry_auto_version" ],

We should either use fields: ['*'], or specify the leaf attributes we want populated : eg: service.node.name, service.environment, service.framework.name, etc. instead of just service.

bryce-b · 2024-09-23T15:24:50Z

x-pack/plugins/observability_solution/apm/server/utils/es_fields_mappings.ts

+      links: [
+        {
+          trace: {
+            // todo(milosz): confirm `span.links.trace.id` format


span links can potentially be an array of many. span.links.trace.id and span.links.span.id will need to be merged and iterated over and added to span.links[]. Below is a screenshot showing their structure in discover.

How did you get those? I see only single values in span.links.span.id and span.links.trace.id when running synthtrace scenario (span_links.ts). Did you make some aggregations?

However, even for single values in span.links.span.id and span.links.trace.id indeed the mapping is incorrect because we expect an array of { trace: { id: string }; span: { id: string }; } so we can get multiple values. With that in mind, I'm not sure how this data will be represented in fields property.

My understanding is that in source fields we might get:

{ span: { link: [ { trace: { id: 'aaa', span: { id: 'aaa' } } }, { trace: { id: 'bbb', span: { id: 'bbb' } } }, { trace: { id: 'ccc', span: { id: 'ccc' } } }, ], }, }

So what format does expect in fields in this situation? I guess this is the problem with all array of object properties, I'm not sure how this is solved 😅

bryce-b · 2024-09-23T17:34:11Z

x-pack/plugins/observability_solution/apm/server/utils/es_fields_mappings.ts

+};
+
+// todo(milosz): test with otel APM POC, mapping format wasn't confirmed with `fields`
+export const spanMapping = (fields: Partial<Record<string, unknown[]>>): Span => {


I noticed that SpanRaw has a couple of additional fields not represented here, such as stacktrace. Do those need to be represented here?

While checking it I found that get_span can give completely different response depending on span. When I first created mapping I based it on response from edge-lite-oblt cluster, now I checked otel-apm-e2e-poc and received different response format. I covered two possible responses in the updated mapping, but I wonder if there are other unknown yet responses for that one. BTW I haven't found stacktrace in neither of two.

get_span in otel-apm-e2e-poc:

get_span in edge-lite-oblt:

jennypavlova · 2024-09-26T13:06:53Z

x-pack/plugins/observability_solution/apm/server/routes/services/get_service_agent.ts

@@ -103,7 +103,7 @@ export async function getServiceAgent({
    return {};
  }

-  const { agent, service, cloud } = normalizeFields(response.hits.hits[0].fields) as ServiceAgent;
+  const { agent, service, cloud } = serviceAgentName(response.hits.hits[0].fields) as ServiceAgent;


In most places where I used as we can replace if with return type in the mapper- so the serviceAgentName return type should be ServiceAgent so we can get rid of the casting.

jennypavlova · 2024-09-26T13:13:23Z

packages/kbn-apm-types/src/es_fields/apm.ts


 export const CHILD_ID = 'child.id';

 export const LOG_LEVEL = 'log.level';
+


The Otel-specific fields (under attributes and resource.attributes should not be part of the new data mapping - we use those fields only in the metadata table (we need to confirm if we show them anywhere else). We should keep the mappers close to the APM types

cc: @AlexanderWert and @felixbarny I remember we discussed that, let me know if I am missing something here

I was expecting the mapping in es_fields_mappings.ts to not produce Record<string, unknown[]> but instead types such as TransactionRaw

jennypavlova · 2024-09-26T13:17:39Z

x-pack/plugins/observability_solution/apm/server/routes/span_links/get_linked_parents.ts

@@ -58,8 +56,7 @@ export async function getLinkedParentsOfSpan({
    },
  });

-  const fields = response.hits.hits[0]?.fields;
-  const fieldsNorm = normalizeFields(fields) as unknown as TransactionRaw | SpanRaw | undefined;
+  const fieldsNorm = linkedParentsOfSpanMapping(response.hits.hits[0]?.fields);


@AlexanderWert I remember we discussed that we don't support the span links ATM so should we find a way to "skip" them or "hide" the tab in case of otel data (Together with @miloszmarcinkowski we found another UI Error we have here because of SpanLink so I am wondering how to handle that in this case)

yes, let's just hide stuff related to Links in the UI and NOT change the query logic for SpanLinks for now

felixbarny · 2024-09-27T07:29:04Z

packages/kbn-apm-types/src/es_fields/apm.ts


 export const CHILD_ID = 'child.id';

 export const LOG_LEVEL = 'log.level';
+


I was expecting the mapping in es_fields_mappings.ts to not produce Record<string, unknown[]> but instead types such as TransactionRaw

felixbarny · 2024-09-27T07:32:06Z

x-pack/plugins/observability_solution/apm/server/utils/es_fields_mappings.ts

+    client: {
+      ip: normalizeValue<string>(fields[CLIENT_IP]),
+    },
+    http: {


We should not be mapping specific request and response headers. It looks like any other header would be lost here. Instead, we should just generically map fields named http.request.headers.*/http.response.headers.*.

felixbarny · 2024-09-27T07:33:33Z

x-pack/plugins/observability_solution/apm/server/utils/es_fields_mappings.ts

+    labels: {
+      some_resource_attribute: normalizeValue<string>(fields[LABEL_SOME_RESOURCE_ATTRIBUTE]),
+    },


Similar to the comment about HTTP headers, we should not hard-code specific labels and instead map any fields matching labels.*.

felixbarny · 2024-09-27T07:33:59Z

x-pack/plugins/observability_solution/apm/server/utils/es_fields_mappings.ts

+  };
+};
+
+export const transactionMapping = (fields: Partial<Record<string, unknown[]>>) => {


Can we make this returning TransactionRaw instead of Record<string, unknown[]>?

Thank you for review @felixbarny!

Record<string, unknown[]> is the parameter type, not return type. If the return type is missing, this means that I couldn't match it with any existing type interface, but typescript does automatically define an implicit type based on the return object format. So, we're left with correct typing at the end.

Today, I have a plan to check all mappings for a possible interface type that will match the returning type.

…ructure-serialization-logic' into bryce-192749-refactor-the-normalize-function-into-per-data-structure-serialization-logic # Conflicts: # x-pack/plugins/observability_solution/apm/server/utils/es_fields_mappings.ts

…tion-into-per-data-structure-serialization-logic added span-link normalization and fixed errors in getTransaction

miloszmarcinkowski · 2024-09-30T10:52:01Z

@bryce-b PR take away:

Created logic to handle Span links. This will be address in second phase, for now use old _source
Missing namespace in Kubernetes icon popover issue

…apm-ui-replacing-_source-with-fields' into 192749-refactor-the-normalize-function-into-per-data-structure-serialization-logic # Conflicts: # x-pack/plugins/observability_solution/apm/common/correlations/types.ts # x-pack/plugins/observability_solution/apm/server/routes/errors/get_error_groups/get_error_group_main_statistics.ts # x-pack/plugins/observability_solution/apm/server/routes/errors/get_error_groups/get_error_sample_details.ts # x-pack/plugins/observability_solution/apm/server/routes/services/get_service_metadata_details.ts

miloszmarcinkowski force-pushed the 192749-refactor-the-normalize-function-into-per-data-structure-serialization-logic branch 5 times, most recently from f70eafd to a85fbf0 Compare September 20, 2024 11:57

bryce-b reviewed Sep 23, 2024

View reviewed changes

miloszmarcinkowski changed the title ~~mapper for 'get metadata for dependency'~~ Otel collector mappings to APM agent Sep 25, 2024

jennypavlova mentioned this pull request Sep 26, 2024

🚧 WIP 🚧 Replace _source in the APM queries elastic/kibana#192608

Closed

miloszmarcinkowski mentioned this pull request Sep 26, 2024

[APM][Otel] Refactor the normalize function into per data structure serialization logic elastic/kibana#192749

Closed

jennypavlova reviewed Sep 26, 2024

View reviewed changes

miloszmarcinkowski force-pushed the 192749-refactor-the-normalize-function-into-per-data-structure-serialization-logic branch from f3c89ca to 20a7b79 Compare September 26, 2024 15:42

felixbarny reviewed Sep 27, 2024

View reviewed changes

miloszmarcinkowski added 15 commits September 27, 2024 13:22

'dependency metadata' mappings

03a86d9

'dependency spans' mapping

2e97e81

span mappings

a6244f5

span links mappings

cb9f1d4

replace field keys with TS constants

0b86fc2

'transaction' mapping

f54e2b3

'trace items' mappings

37dd831

'error group main statistics' mapping

8e55406

'error sample details' mapping

0cabafb

'service metadata' mapping

f5e627c

'metadata and service version' mappings

c52ddfd

'service agent name' mappings

e77d9a4

add fields to Span mapping

f8f9b78

remove unused properties in Span mapping

3eac633

improve name consistency in mapping functions

d467a38

miloszmarcinkowski added 2 commits September 27, 2024 13:23

remove unused fields from 'service instance metadata details'

ac07c73

add check against undefined

8666c00

miloszmarcinkowski force-pushed the 192749-refactor-the-normalize-function-into-per-data-structure-serialization-logic branch from 294ac0e to 8666c00 Compare September 27, 2024 15:34

bryce-b and others added 9 commits September 27, 2024 10:00

added span-link normalization and fixed errors in getTransaction

06545b5

fix service metadata endpoints

a08ef93

PR feedback fix

f156559

added mapping for metadata_details

2ef8527

fixed kubernetes check in metadata mappings

004f587

use one mapping for metadata details

354d821

update 'todo' and processor_name typo

aebea67

Merge pull request #2 from bryce-b/192749-refactor-the-normalize-func…

b4c5a95

…tion-into-per-data-structure-serialization-logic added span-link normalization and fixed errors in getTransaction

miloszmarcinkowski marked this pull request as ready for review September 30, 2024 11:24

add container mapping

e4fac40

jennypavlova merged commit 634b899 into jennypavlova:192606-poc-otel-data-with-apm-ui-replacing-_source-with-fields Sep 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Otel collector mappings to APM agent #6

Otel collector mappings to APM agent #6

miloszmarcinkowski commented Sep 18, 2024 •

edited

Loading

bryce-b Sep 23, 2024

bryce-b Sep 23, 2024

miloszmarcinkowski Sep 25, 2024 •

edited

Loading

bryce-b Sep 23, 2024

miloszmarcinkowski Sep 23, 2024

bryce-b Sep 24, 2024

bryce-b Sep 24, 2024 •

edited

Loading

bryce-b Sep 23, 2024

miloszmarcinkowski Sep 25, 2024

bryce-b Sep 23, 2024

miloszmarcinkowski Sep 25, 2024

miloszmarcinkowski Sep 25, 2024

jennypavlova Sep 26, 2024

jennypavlova Sep 26, 2024

felixbarny Sep 27, 2024

jennypavlova Sep 26, 2024

AlexanderWert Sep 30, 2024

felixbarny Sep 27, 2024

felixbarny Sep 27, 2024

felixbarny Sep 27, 2024

felixbarny Sep 27, 2024

miloszmarcinkowski Sep 27, 2024

miloszmarcinkowski commented Sep 30, 2024 •

edited

Loading


		export const CHILD_ID = 'child.id';

		export const LOG_LEVEL = 'log.level';

Otel collector mappings to APM agent #6

Otel collector mappings to APM agent #6

Conversation

miloszmarcinkowski commented Sep 18, 2024 • edited Loading

Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miloszmarcinkowski Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryce-b Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miloszmarcinkowski commented Sep 30, 2024 • edited Loading

miloszmarcinkowski commented Sep 18, 2024 •

edited

Loading

miloszmarcinkowski Sep 25, 2024 •

edited

Loading

bryce-b Sep 24, 2024 •

edited

Loading

miloszmarcinkowski commented Sep 30, 2024 •

edited

Loading