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

Faas trace attributes: requirement levels need revision #2599

Closed
lmolkova opened this issue Jun 2, 2022 · 5 comments · Fixed by #2627
Closed

Faas trace attributes: requirement levels need revision #2599

lmolkova opened this issue Jun 2, 2022 · 5 comments · Fixed by #2627
Assignees
Labels
area:semantic-conventions Related to semantic conventions spec:trace Related to the specification/trace directory

Comments

@lmolkova
Copy link
Contributor

lmolkova commented Jun 2, 2022

faas trace semantic conventions define multiple attributes as required and some of these choices seem controversial:

  • faas.document.time - it's required, but it's likely the same time as span start time. Is this attribute really needed? Should it be optional ?
  • faas.time - same concern - it should match span start time, why another one is needed and can it be optional?
  • faas.invoked_name span attribute that SHOULD match faas.name resource attribute, seems like duplication, why faas.invoked_name is needed and can it be optional?
  • invoked_provider and invoked_region - should they be resource attributes (they are static)? should they be recommended instead?
@lmolkova lmolkova added the spec:trace Related to the specification/trace directory label Jun 2, 2022
@Oberon00
Copy link
Member

Oberon00 commented Jun 2, 2022

faas.invoked_name span attribute that SHOULD match faas.name resource attribute, seems like duplication, why faas.invoked_name is needed and can it be optional?

It SHOULD match the remote side's faas.name, but that one might not be instrumented, or an error might occur with reaching it or transmission of the remote side's trace. Same with the other invoked_* attributes.
They cannot be resource attributes because you may call more than one different function (the same way an outgoing http.url cannot be a resource attribute).

For the first two (faas.time, faas.document.tme), I'd agree to make them "recommended".

@Oberon00 Oberon00 added the area:semantic-conventions Related to semantic conventions label Jun 2, 2022
@lmolkova
Copy link
Contributor Author

lmolkova commented Jun 3, 2022

Thanks for the clarification!
At least in Azure Functions world clients never know if they're calling an Azure Function or anything else. It sounds like the whole Outgoing Invocations is in manual instrumentation land or covers a few edge cases. Requiring anything, in this case, seems difficult.

@Oberon00
Copy link
Member

Oberon00 commented Jun 7, 2022

It sounds like the whole Outgoing Invocations is in manual instrumentation land or covers a few edge cases.

No, this was introduced for AWS Lambda, where you have an API call to invoke a function (that can also be automatically instrumented with a hook mechanism). https://docs.aws.amazon.com/lambda/latest/dg/API_Invoke.html It seems that other cloud providers don't provide this mechanism though (except maybe Alibaba which seems to be, so it can't be sensibly set there (except manually indeed).

EDIT: Actually, Alibaba also seems to have that feature: https://www.alibabacloud.com/help/en/function-compute/latest/api-doc-fc-open-2021-04-06-api-doc-invokefunction

@jack-berg
Copy link
Member

The language is also inconsistent in the faas trace semantic convention spec. Here it says that "it is recommended to set the following attributes", and proceeds to list a table of attributes which are all marked as "required".

@Oberon00
Copy link
Member

Oberon00 commented Jun 9, 2022

Technically this would have the meaning that you SHOULD apply this semantic convention group. Which means you are allowed to leave it out completely, but if you choose to use the attributes, then you MUST fulfill the requirement levels of the table. Which allows having no special support for that trigger and still conforming to FaaS conventions, but if you do implement special support, some attributes are required.
(Not sure if the concrete required attributes make sense though)

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 spec:trace Related to the specification/trace directory
Projects
None yet
4 participants