-
Notifications
You must be signed in to change notification settings - Fork 291
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
Added support for semantic convention 1_20 and 1_23 #1904
Conversation
- attribute: server.address | ||
regex: ^search-[a-zA-Z0-9-]+-[a-zA-Z0-9]+\.((aos\.[a-zA-Z0-9-]+\.on\.aws)|(([a-zA-Z0-9-]+)\.(es|aos)\.amazonaws\.com))$ |
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.
ElasticSearch follows database conventions, so this file requires rules for 1.20 and 1.23 conventions.
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.
Added changes
- attribute: rpc.service | ||
anyOf: [ "Lambda" ] |
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.
Why rpc.service=Lambda
? I do not see this in the conventions, so I do not expect that instrumentation will add the rpc.service
attribute.
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.
Added changes
- attribute: aws.region | ||
present: true | ||
- attribute: aws.account.id | ||
present: true |
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 do not see aws.region
or aws.account.id
in the specification. What instrumentation are you using that adds these attributes?
@@ -1,13 +1,24 @@ | |||
relationships: | |||
- name: extServiceCallsInfraAwsSqsQueue | |||
- name: extServiceCallsInfraAwsSqsQueue1_20 |
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.
This rule does not look like 1.20 conventions, so this name is not accurate. 1.20 conventions do not have an aws.region
or aws.account.id
attribute.
attribute: messaging.destination.name | ||
|
||
- name: extServiceCallsInfraAwsSqsQueue1_23 |
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.
There is nothing in this rule that indicates it follow 1.23 conventions. All the attributes you are relying on are present in 1.20.
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.
How does this rule relate to the AWSLAMBDAALIAS rule in this other PR https://github.com/newrelic/entity-definitions/pull/1906/files#diff-328a5048f522a203ae3b2281d21e89d0adfa40bd94b88dd9c5deb31bc17701d7L1?
Is it expected that both an AWSLAMBDAFUNCTION and AWSLAMBDAALIAS relationship be created? If not, when would I expect one over the other?
We are closing this PR and will create two separate PRs: one for clean changes and one for the issue-related ones. |
Relevant information
Added support for semantic convention 1_20 and 1_23
Checklist
identifier
will be unique and valid.