Only overwrite service.version if service.name is also set

[tobiasstadler]

Motivation/summary

For metricsets the service.version is only overwritten if also service.name is given (see #6407 (comment)). It should be the same for transactions, errors, ...

Checklist

• Update CHANGELOG.asciidoc
• Update package changelog.yml (only if changes to apmpackage have been made)
• Documentation has been updated

For functional changes, consider:

• Is it observable through the addition of either logging or metrics?
• Is its use being published in telemetry to enable product improvement?
• Have system tests been added to avoid regression?

How to test these changes

Ingest a transaction that only contains a service.version, but no service.name

[ghost]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2022-02-15T12:20:11.319+0000

• Duration: 61 min 31 sec

Test stats 🧪

Test	Results
Failed	0
Passed	5635
Skipped	19
Total	5654

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

• /hey-apm : Run the hey-apm benchmark.

• /package : Generate and publish the docker images.

• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[mergify]

This pull request does not have a backport label. Could you fix it @tobiasstadler? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-7.x is the label to automatically backport to the 7.x branch.
• backport-7./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

[axw]

LGTM, thanks!

[axw]

/test

[tobiasstadler]

@axw I fixed the tests. make test and make system-test finished successfully on my machine.

[tobiasstadler]

Since this is some kind of bug fix, should it be back ported to 8.1?

[axw]

@tobiasstadler we have already frozen 8.1. Although it's arguable a bug fix, I'd prefer that it waits for 8.2. Is it causing any issues for you?

[tobiasstadler]

No, 8.2 is fine.

[tobiasstadler]

run elasticsearch-ci/docs

[tobiasstadler]

Thank You!

[stuartnelson3]

testing this was unsuccessful with 772e8c1

1. start elasticsearch/kibana
2. start apm-server
3. ingest span + transaction with the following program:

package main

import (
	"context"
	"fmt"
	"os"
	"time"

	"go.elastic.co/apm/v2"
)

func main() {
	version := "undefined"
	if len(os.Args) > 1 {
		version = os.Args[1]
	}
	name := fmt.Sprintf("apm-server-%s", version)

	tracer, err := apm.NewTracer("", "1.0.0")
	if err != nil {
		panic(err)
	}
	tx := tracer.StartTransaction("name", "type")
	ctx := apm.ContextWithTransaction(context.Background(), tx)
	span, ctx := apm.StartSpan(ctx, "name", "type")
	span.End()
	tx.End()
	<-time.After(time.Second)
	tracer.Flush(nil)
	fmt.Printf("%s: %+v\n", name, tracer.Stats())
}

4. check the transaction document has no service.version set. In the console:

GET traces-apm-default/_search
{
  "sort" : [{ "@timestamp" : "desc" }]
}

service.name seems to have been set to the default value (the binary name) from the agent, and therefore service.version was also set.
I attempted to injest an ndjson document without a service.name set, but the apm server rejected it (validation error: metadata: service: 'name': validation rule 'minLength(1)' violated)

[axw]

@stuartnelson3 I'm not sure that your tests cover Tobias's changes.

The changes are related to overriding service.name and service.version at the event level, which the Go agent's API doesn't support. Your test above only affects the service.name and service.version set at the stream (metadata) level.

I've verified this with 8.2.0-BC1, using https://github.com/elastic/apm-server/blob/main/testdata/intake-v2/transactions_spans.ndjson#L2.

The first transaction overrides service.name, but does not specify service.version. I confirmed the service.name field is set to "serviceabc" but there is no service.version field set.

I then modified the document to override service.version but not service.name, and confirmed that neither are used and that service.name and service.version from metadata are both used.

Finally, I confirmed that if both service.name and service.version are specified in the overrides, both are used.