Add support for semantic convention 1_20 and 1_23 #1903

Sanjalee-NewRelic · 2025-02-07T05:35:21Z

Relevant information

Modified relationships to support otel semantic conventions version 1.20 and 1.23

Checklist

I've read the guidelines and understand the acceptance criteria.
The value of the attribute marked as identifier will be unique and valid.
I've confirmed that my entity type wasn't already defined. If it is I'm providing an explanation above.

alanwest · 2025-02-07T18:33:58Z

relationships/synthesis/EXT-SERVICE-to-INFRA-MQBROKER.yml

+      - attribute: span.kind
+        anyOf: [ "producer" ]


With regards to span.kind the conventions state

A producer of a message should set the span kind to PRODUCER unless it synchronously waits for a response: then it should use CLIENT. The processor of the message should set the kind to CONSUMER, unless it always sends back a reply that is directed to the producer of the message (as opposed to e.g., a queue on which the producer happens to listen): then it should use SERVER.

All span.kind conditions should be updated as follows:

For relationshipType=PRODUCES

- attribute: span.kind anyOf: [ "producer", "client" ]

For relationshipType=CONSUMES

- attribute: span.kind anyOf: [ "consumer", "server" ]

alanwest · 2025-02-07T18:41:29Z

relationships/synthesis/EXT-SERVICE-to-INFRA-MQBROKER.yml

Should the name of this file be EXT-SERVICE-to-INFRA-AWSMQBROKER? Or do we anticipate that in the future this file will contain relationship definitions for MQ Brokers other than AWS?

Please let's keep it scoped to the domain/types involved, we can create another file if we need to extend beyond of AWS.

@Sanjalee-NewRelic please rename the file to be specific to AWS EXT-SERVICE-to-INFRA-AWSMQBROKER.yml

alanwest · 2025-02-07T18:45:33Z

relationships/synthesis/EXT-SERVICE-to-INFRA-MQBROKER.yml

+    conditions:
+      - attribute: eventType
+        anyOf: [ "Span" ]
+      - attribute: span.kind
+        anyOf: [ "consumer" ]
+      - attribute: messaging.system
+        anyOf: [ "rabbitmq", "activemq" ]
+      - attribute: server.address
+        present: true


The conditions for all the rules in this file are not sufficient for constraining them to target only EXT-SERVICE entities.

I suggest we add these additional conditions to all rules similar to what we have done for our infra-host to ext-service relationship:

entity-definitions/relationships/synthesis/INFRA-HOST-to-EXT-SERVICE.yml

Lines 9 to 12 in dbf47cc

- attribute: newrelic.source

anyOf: [ "api.traces.otlp" ]

- attribute: entity.type

anyOf: [ "SERVICE" ]

Constraining both on newrelic.source and entity.type give us higher confidence that this rule specifically targets OpenTelemetry instrumented services.

alanwest · 2025-02-07T18:58:35Z

relationships/synthesis/EXT-SERVICE-to-INFRA-MQBROKER.yml

+      target:
+        lookupGuid:
+          candidateCategory: AWSMQBROKER
+          fields:
+            - field: endpoint
+              attribute: net.peer.name


Currently, this lookup onMiss will create an an uninstrumented MQBROKER entity.

entity-definitions/relationships/candidates/AWSMQBROKER.yml

Lines 13 to 16 in dbf47cc

onMiss:

action: CREATE_UNINSTRUMENTED

uninstrumented:

type: MQBROKER

In the future if we add new relationships supporting messaging systems besides AWS (e.g., RabbitMQ hosted elsewhere), will this rule cause us to falsely create uninstrumented relationships?

If so, then we may need to constrain this rule to be AWS specific. Maybe like so?

conditions: - attribute: net.peer.name regex: <regex that identifies this is AWS>

alanwest · 2025-02-07T19:50:35Z

relationships/synthesis/EXT-SERVICE-to-INFRA-MQBROKER.yml

+    origins:
+      - Distributed Tracing
+      - OpenTelemetry


It's not entirely clear what origins should be used here. In reviewing other rules it seems we are inconsistent.

For example, the infra-host to ext-service rule derived from span data only uses an origin of Distributed Tracing:

entity-definitions/relationships/synthesis/INFRA-HOST-to-EXT-SERVICE.yml

Lines 4 to 5 in dbf47cc

origins:

- Distributed Tracing

Whereas the the infra-host to ext-service rule derived from metric data only uses an origin of OpenTelemetry:

entity-definitions/relationships/synthesis/INFRA-HOST-to-EXT-SERVICE.yml

Lines 37 to 38 in dbf47cc

origins:

- OpenTelemetry

I can see the following origins being relevant to the rules in this PR:

Distributed Tracing because these rules target span data.

OpenTelemetry because these rules are meant to target OpenTelemetry instrumented services

AWS Integration because these rules target our AWS Metric Streams integration

@otaviocarvalho can you provide some guidance here? From what I can tell these origins have been inconsistently applied across other relationship rules, so it doesn't seem like they're very important.

All three origins are relevant to the domain of these rules. Is the practice that we should list all relevant origins? Or does it not really matter?

alanwest · 2025-02-07T19:59:14Z

relationships/synthesis/EXT-SERVICE-to-KAFKATOPIC.yml

+            - field: topic
+              attribute: messaging.destination.name


messaging.destination.name is not guaranteed to exist. If these rules require it be present then it should be added to the conditions

conditions: ... - attribute: messaging.destination.name present: true

alanwest · 2025-02-07T20:13:54Z

relationships/synthesis/EXT-SERVICE-to-KAFKATOPIC.yml

+                attribute: net.peer.name            
+                regex: "^[^\\.]+\\.([^\\.]+)\\.[^\\.]+\\.[^\\.]+\\.kafka\\.[^\\.]+\\.amazonaws\\.com:[0-9]+.*"


Similar to my question above, should this rule be constrained by adding a condition that makes it specific to AWS?

Probably being too pragmatic, but if the domain includes amazonaws...com there is no way it will not be specific to AWS. Alternatively, you can add and check a specific tag for this purpose.

otaviocarvalho

LGTM. This PR needs detailed attention from Entity Platform, to make sure we are disabling all sub-rules before it goes into production (to avoid enabling it automatically by mistake).

alanwest · 2025-02-10T17:16:15Z

relationships/synthesis/EXT-SERVICE-to-MEMCACHED.yml

+    conditions:
+      - attribute: eventType
+        anyOf: [ "Span" ]
+      - attribute: db.system
+        anyOf: [ "memcached" ]
+      - attribute: net.peer.name
+        present: true
+      - attribute: net.peer.port
+        present: false


@Sanjalee-NewRelic These rules for memcached should also be constrained by entity.type and newrelic.source

entity-definitions/relationships/synthesis/INFRA-HOST-to-EXT-SERVICE.yml

Lines 9 to 12 in dbf47cc

- attribute: newrelic.source

anyOf: [ "api.traces.otlp" ]

- attribute: entity.type

anyOf: [ "SERVICE" ]

Added the conditions @alanwest

alanwest

This PR looks good now. @Sanjalee-NewRelic please handle this last piece of feedback #1903 (comment)

alanwest · 2025-02-12T00:10:37Z

relationships/synthesis/EXT-SERVICE-to-MEMCACHED.yml

+      - attribute: db.system
+        anyOf: [ "memcached" ]


@Sanjalee-NewRelic After reviewing the rules for redis, it looks like they are restricted to AWS.

See: https://github.com/newrelic/entity-definitions/pull/1904/files#diff-f8b949e2507623f79e34ed7186ae981c06082c0e27013bce60088e0d0de9beb2R52

I think it makes sense to do the same for memcached, right?

Something like

- attribute: server.address regex: <regex constraining this to AWS>

Yes, This can be done. Made the change.

…Sanjalee-NewRelic/entity-definitions into Part2-Update-Attributes-As-Per-Semantic-Convention

fix: Add support for semantic convention 1_20 and 1_23

8269617

entityBot approved these changes Feb 7, 2025

View reviewed changes

github-actions bot previously approved these changes Feb 7, 2025

View reviewed changes

Sanjalee-NewRelic changed the title ~~fix: Add support for semantic convention 1_20 and 1_23~~ <DO NOT MERGE> Add support for semantic convention 1_20 and 1_23 Feb 7, 2025

alanwest reviewed Feb 7, 2025

View reviewed changes

otaviocarvalho previously approved these changes Feb 10, 2025

View reviewed changes

fix: Made relationship files more specific to AWS

555c259

Sanjalee-NewRelic dismissed stale reviews from otaviocarvalho and github-actions[bot] via 555c259 February 10, 2025 10:58

entityBot approved these changes Feb 10, 2025

View reviewed changes

github-actions bot previously approved these changes Feb 10, 2025

View reviewed changes

fix: Add Opentelemetry origin

37c7f98

Sanjalee-NewRelic dismissed github-actions[bot]’s stale review via 37c7f98 February 10, 2025 13:59

entityBot approved these changes Feb 10, 2025

View reviewed changes

github-actions bot previously approved these changes Feb 10, 2025

View reviewed changes

alanwest reviewed Feb 10, 2025

View reviewed changes

fix: Add otel service conditions

c84e2f0

Sanjalee-NewRelic dismissed github-actions[bot]’s stale review via c84e2f0 February 10, 2025 17:33

entityBot approved these changes Feb 10, 2025

View reviewed changes

github-actions bot previously approved these changes Feb 10, 2025

View reviewed changes

alanwest approved these changes Feb 11, 2025

View reviewed changes

fix: Renamed file

a5ad237

Sanjalee-NewRelic dismissed github-actions[bot]’s stale review via a5ad237 February 11, 2025 03:38

entityBot approved these changes Feb 11, 2025

View reviewed changes

github-actions bot previously approved these changes Feb 11, 2025

View reviewed changes

Sanjalee-NewRelic changed the title ~~<DO NOT MERGE> Add support for semantic convention 1_20 and 1_23~~ Add support for semantic convention 1_20 and 1_23 Feb 11, 2025

alanwest reviewed Feb 12, 2025

View reviewed changes

fix: Add aws specific condition for mrmcached

a032eff

Sanjalee-NewRelic dismissed github-actions[bot]’s stale review via a032eff February 12, 2025 07:05

entityBot approved these changes Feb 12, 2025

View reviewed changes

github-actions bot approved these changes Feb 12, 2025

View reviewed changes

Pull latest code from main Merge branch 'main' of https://github.com/…

769822a

…Sanjalee-NewRelic/entity-definitions into Part2-Update-Attributes-As-Per-Semantic-Convention

entityBot approved these changes Feb 12, 2025

View reviewed changes

github-actions bot approved these changes Feb 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for semantic convention 1_20 and 1_23 #1903

Add support for semantic convention 1_20 and 1_23 #1903

Sanjalee-NewRelic commented Feb 7, 2025 •

edited

Loading

alanwest Feb 7, 2025

alanwest Feb 7, 2025

otaviocarvalho Feb 10, 2025

alanwest Feb 10, 2025 •

edited

Loading

alanwest Feb 7, 2025

alanwest Feb 7, 2025

alanwest Feb 7, 2025

alanwest Feb 10, 2025

alanwest Feb 7, 2025

alanwest Feb 7, 2025

otaviocarvalho Feb 10, 2025

otaviocarvalho left a comment

alanwest Feb 10, 2025

Sanjalee-NewRelic Feb 10, 2025

alanwest left a comment •

edited

Loading

alanwest Feb 12, 2025 •

edited

Loading

Sanjalee-NewRelic Feb 12, 2025

	- attribute: newrelic.source
	anyOf: [ "api.traces.otlp" ]
	- attribute: entity.type
	anyOf: [ "SERVICE" ]

	onMiss:
	action: CREATE_UNINSTRUMENTED
	uninstrumented:
	type: MQBROKER

		attribute: net.peer.name
		regex: "^[^\\.]+\\.([^\\.]+)\\.[^\\.]+\\.[^\\.]+\\.kafka\\.[^\\.]+\\.amazonaws\\.com:[0-9]+.*"

Add support for semantic convention 1_20 and 1_23 #1903

Are you sure you want to change the base?

Add support for semantic convention 1_20 and 1_23 #1903

Conversation

Sanjalee-NewRelic commented Feb 7, 2025 • edited Loading

Relevant information

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanwest Feb 10, 2025 • 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

otaviocarvalho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alanwest left a comment • edited Loading

Choose a reason for hiding this comment

alanwest Feb 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sanjalee-NewRelic commented Feb 7, 2025 •

edited

Loading

alanwest Feb 10, 2025 •

edited

Loading

alanwest left a comment •

edited

Loading

alanwest Feb 12, 2025 •

edited

Loading