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

Expand scope of faas.id to cloud.resource_id #3188

Merged
merged 12 commits into from
Feb 27, 2023

Conversation

tylerbenson
Copy link
Member

@tylerbenson tylerbenson commented Feb 8, 2023

Changes

As per the discussion on #3188, we determined it is better to move rather than remove as this attribute is still important for some systems despite it containing duplicated information.

semantic_conventions/resource/faas.yaml Outdated Show resolved Hide resolved
semantic_conventions/resource/faas.yaml Show resolved Hide resolved
semantic_conventions/resource/faas.yaml Show resolved Hide resolved
@arminru arminru added area:semantic-conventions Related to semantic conventions spec:resource Related to the specification/resource directory spec:trace Related to the specification/trace directory spec:metrics Related to the specification/metrics directory labels Feb 9, 2023
@cartersocha
Copy link

CC: @lmolkova, @dashpole, @reyang. The unique resource ID conversation(s) above in particular will need cloud SMEs. While changing the faas spec in general we've concluded some of changes will be better placed in cloud requirements rather than FAAS.

@tylerbenson
Copy link
Member Author

tylerbenson commented Feb 14, 2023

FYI. I've pulled the first two commits out into a separate PR #3209. I'll try moving faas.id over to the cloud spec as cloud.resource_id instead of just removing it in this PR. Thanks everyone for the feedback.

tylerbenson added a commit to tylerbenson/opentelemetry-specification that referenced this pull request Feb 14, 2023
As per the discussion on open-telemetry#3188, we determined it is better to move rather than remove as this attribute is still important for some systems despite it containing duplicated information.
@tylerbenson tylerbenson changed the title FAAS SIG spec changes Expand scope of faas.id to cloud.resource_id Feb 14, 2023
As per the discussion on open-telemetry#3188, we determined it is better to move rather than remove as this attribute is still important for some systems despite it containing duplicated information.
tylerbenson and others added 3 commits February 16, 2023 11:53
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@carlosalberto
Copy link
Contributor

Hey @Oberon00

I'm OK with leaving the primary definition here, but we should still ref this from the FaaS conventions IMHO as it's quite important.

You mean adding a reference/link from the FaaS semconv table to this section?

tylerbenson and others added 2 commits February 16, 2023 17:08
Co-authored-by: Christian Neumüller <christian+github@neumueller.me>
@carlosalberto
Copy link
Contributor

carlosalberto commented Feb 17, 2023

Btw @Oberon00

Following on this: once we decide how to proceed with open-telemetry/oteps#222 we should also double check expectations here regarding ECS.

FWIW I think ECS is a better, clearer, cleaner semconv reference, with faas with IMHO the exception (e.g. faas.execution created lots of confusion among users)

@Oberon00
Copy link
Member

You mean adding a reference/link from the FaaS semconv table to this section?

Exactly (using the ref mechanism in the YAML definition of the FaaS table)

@Oberon00
Copy link
Member

Oberon00 commented Feb 20, 2023

Following on this: once we decide how to proceed with open-telemetry/oteps#222 we should also double check expectations here regarding ECS.

I think you should decide on this rather sooner than later otherwise lots of work on the semantic conventions (such as this PR) and also potentially the tooling may go to waste.

# Conflicts:
#	schemas/1.19.0
#	specification/resource/semantic_conventions/faas.md
#	specification/trace/semantic_conventions/faas.md
#	specification/trace/semantic_conventions/instrumentation/aws-lambda.md
@carlosalberto carlosalberto merged commit c00654c into open-telemetry:main Feb 27, 2023
@tylerbenson tylerbenson deleted the tyler/faas branch February 27, 2023 20:50
jsuereth pushed a commit to jsuereth/opentelemetry-specification that referenced this pull request Feb 28, 2023
## Changes
As per the discussion on open-telemetry#3188, we determined it is better to move
rather than remove as this attribute is still important for some systems
despite it containing duplicated information.
Oberon00 added a commit to dynatrace-oss-contrib/opentelemetry-specification that referenced this pull request Mar 21, 2023
These places were removed when open-telemetry#3188 in its original form removed
faas.id without replacement and weren't added back after changing
it to introduce cloud.resource_id instead.
Oberon00 added a commit to dynatrace-oss-contrib/opentelemetry-specification that referenced this pull request Mar 21, 2023
These places were removed when open-telemetry#3188 in its original form removed
faas.id without replacement and weren't added back after changing
it to introduce cloud.resource_id instead.
carlosalberto pushed a commit that referenced this pull request Apr 11, 2023
These places were removed when #3188 in its original form removed
faas.id without replacement and weren't added back after changing it to
introduce cloud.resource_id instead.

Editorial change, no CHANGELOG or issue.
codeboten pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this pull request Oct 6, 2023
…26486)

Context:
GoogleCloudPlatform/opentelemetry-operations-go#679

faas.id was removed from the semantic conventions:
open-telemetry/opentelemetry-specification#3188

We were previously using it improperly to store the instance id of the
faas, which should be mapped to faas.instance instead.

---------

Co-authored-by: Alex Boten <aboten@lightstep.com>
jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this pull request Nov 12, 2023
…pen-telemetry#26486)

Context:
GoogleCloudPlatform/opentelemetry-operations-go#679

faas.id was removed from the semantic conventions:
open-telemetry/opentelemetry-specification#3188

We were previously using it improperly to store the instance id of the
faas, which should be mapped to faas.instance instead.

---------

Co-authored-by: Alex Boten <aboten@lightstep.com>
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:metrics Related to the specification/metrics directory spec:resource Related to the specification/resource directory spec:trace Related to the specification/trace directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.