use faas.instance instead of faas.id in gcp detector

[dashpole]

Context: GoogleCloudPlatform/opentelemetry-operations-go#679

faas.id is being removed in newer versions of the semantic conventions. faas.id is meant to be the URI of the google cloud resource (e.g. projects/my-project/locations/us-central1/functions/my-function), but we were using it to store the instance id.

The correct resource attribute to use in this case is the faas.instance attribute, which has the description:

The execution environment ID as a string, that will be potentially reused for other invocations to the same function/function version.

That matches what the instance ID is in GAE, Google Cloud Run, and Google Cloud Functions.

Since faas.id has been removed, we will need to undergo a breaking change regardless. It is probably better to do so now, rather than wait until contrib is updated.

[codecov]

Codecov Report

Merging #4198 (22d9b30) into main (53f4e7e) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@          Coverage Diff          @@
##            main   #4198   +/-   ##
=====================================
  Coverage   79.4%   79.4%           
=====================================
  Files        166     166           
  Lines      10363   10363           
=====================================
+ Hits        8230    8233    +3     
+ Misses      1998    1996    -2     
+ Partials     135     134    -1     

Files Changed	Coverage
detectors/gcp/detector.go	100.0%

[MrAlias]

Given this is a stable detector, is there something we need to do according to the semconv world to provide backward compatibility here?

[dashpole]

I think this would be covered by https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/versioning-and-stability.md#semantic-conventions-stability. It says:

Changes to semantic conventions in this specification are allowed, provided that the changes can be described by schema files.

That would seem to imply we are required to publish a schema file with this detector describing the change. But then it also contradicts that with:

There is a moratorium on relying on schema transformations for telemetry stability.

Each language implementation MUST take these versioning and stability requirements, and produce a language-specific document which details how these requirements will be met. This document SHALL be placed in the root of each repo and named VERSIONING.

It does say we are bound by our VERSIONING.md file in otel-go. The otel-go versioning policy doesn't seem to say anything about detectors except that they are a module, and follow semver.

We shouldn't have made this package stable given it implements the detection experimental semantic conventions, but that ship sailed... Options I can think of:

1. Stay on the current version of the semantic conventions ~forever
2. Release a v2 of the package with the change
3. Deprecate the existing NewDetector function, and add a new function that uses newer semantic conventions
4. Make the breaking change with the rationale that it maintains API stability.

[MrAlias]

Moving forward with option (4) based on SIG discussion

[dashpole]

Discussed in the Sig meeting. We will go with option 4. We need to update to not break users: #4261 (comment).

[dustinmoris]

What's delaying the merge of this PR at this point?