Skip to content
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

Consider making faas.id a span attribute instead of a resource attribute #1261

Open
mateuszrzeszutek opened this issue Nov 26, 2020 · 16 comments
Assignees
Labels
area:semantic-conventions Related to semantic conventions release:after-ga Not required before GA release, and not going to work on before GA spec:resource Related to the specification/resource directory

Comments

@mateuszrzeszutek
Copy link
Member

What are you trying to achieve?

Currently faas.id is a resource attribute. In AWS Lambda functions its value (lambda function ARN) is only available as a property of the AWS lambda context passed to the function, so it's impossible to detect this part just from the environment.

@mateuszrzeszutek mateuszrzeszutek added the spec:resource Related to the specification/resource directory label Nov 26, 2020
@Oberon00
Copy link
Member

Oberon00 commented Nov 26, 2020

Indeed all the FaaS resource attributes also caused problems at Dynatrace, to the point where we patched our SDKs to allow adding resource attributes until before the first span is sent.
So I would agree with changing all/most FaaS attributes to span attributes, or (preferably) allowing them to be set as either resource or span attributes.
From a technical perspective, since we need to flush after each incoming lambda invocation (since we might be suspended afterwards) there is usually a 1:1 ratio of incoming lambda spans and resource messages exported anyway, so we don't even conserve bandwidth on AWS by sending these as resources.

@Oberon00 Oberon00 added the area:semantic-conventions Related to semantic conventions label Nov 26, 2020
@Oberon00
Copy link
Member

CC @thisthat @discostu105

@mateuszrzeszutek
Copy link
Member Author

allowing them to be set as either resource or span attributes.

That would be perfect for all/most cloud/faas attributes. For example, if I wanted to add cloud.account.id on AWS Lambda there is no other way (I think?) then extracting it from the ARN, which again makes it impossible to add it as a resource attribute.

@Oberon00 Oberon00 changed the title Convider making faas.id a span attribute instead Cosider making faas.id a span attribute instead of a resource attribute Nov 26, 2020
@mateuszrzeszutek mateuszrzeszutek changed the title Cosider making faas.id a span attribute instead of a resource attribute Consider making faas.id a span attribute instead of a resource attribute Nov 27, 2020
@mateuszrzeszutek
Copy link
Member Author

Can somebody provide insight on how should we deal with the yaml files? Should I just duplicate the attributes in resources & traces?

@Oberon00
Copy link
Member

I would say we keep it as resource but add a note in the markdown that they may also be sent as span attributes. We may later want to add support for the generator to mark an attribute as being able to be put on a span.

@andrewhsu andrewhsu added priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA labels Dec 1, 2020
@andrewhsu andrewhsu assigned andrewhsu and carlosalberto and unassigned andrewhsu Dec 1, 2020
@Oberon00
Copy link
Member

Oberon00 commented Dec 9, 2020

BTW @anuraaga, @alolita or someone from else AWS: are there any plans that AWS Lambda will improve the situation here? It is quite inconvenient that the ARN is not available before the first request is handled. For example, you can not report JVM memory metrics associated with an ARN before the first request hits the lambda (e.g. provisioned concurrency cases).

@anuraaga
Copy link
Contributor

@Oberon00 Thanks for the callout - I tried filing a ticket with the lambda team to see if they can provide ARN as an environment variable. We'll see how it goes, I wouldn't expect it anytime soon though.

@anuraaga
Copy link
Contributor

anuraaga commented Dec 14, 2020

Though after reading #1280 I have a feeling my ticket will be rejected :) I guess the same execution environment can be used with different ARN's because of the aliases.

@Oberon00
Copy link
Member

Oberon00 commented Dec 14, 2020

@anuraaga The (primary?) function name is already known from an environment variable on AWS, as is the region. The only thing missing to form an ARN is the account ID.

@Oberon00
Copy link
Member

Oberon00 commented Jan 11, 2021

@anuraaga Any news from the AWS side, on the availability of the account ID or full ARN at startup?

@anuraaga
Copy link
Contributor

@Oberon00 It's a feature request, and apparently there are some corner cases so not sure if / when it'll be implemented unfortunately.

@carlosalberto
Copy link
Contributor

carlosalberto commented Jan 21, 2021

I would say we keep it as resource but add a note in the markdown that they may also be sent as span attributes. We may later want to add support for the generator to mark an attribute as being able to be put on a span.

+1 to this. Shall we proceed? I'd like to have this assigned (or work on it myself) so we can make progress ;)

@iNikem
Copy link
Contributor

iNikem commented Jan 25, 2021

Just a note: this problem can also be solved by doing #1298

@andrewhsu
Copy link
Member

discussed at the maintainers mtg today...desire is to move this to after ga @Oberon00 comment if you would see this as required for ga

@andrewhsu andrewhsu added release:after-ga Not required before GA release, and not going to work on before GA and removed priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA labels Jan 25, 2021
@Oberon00
Copy link
Member

I'm fine with after-ga.

@Oberon00
Copy link
Member

Oberon00 commented Jul 28, 2022

I believe this is actually kind of resolved now, with faas.id being explicitly allowed as a span attribute as well.

This was done at https://github.com/open-telemetry/opentelemetry-specification/pull/1781/files#diff-1ef1d72af0336c2273408806365fd2621ab7e6d65b664798ce1c842250ce0e8eR33-R37 and again clarified/adapted further in #2502

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions release:after-ga Not required before GA release, and not going to work on before GA spec:resource Related to the specification/resource directory
Projects
None yet
6 participants