fix(test): resolve flaky TestSourcemapFetcher by avoiding fields parsing issue

[Copilot]

TestSourcemapFetcher and TestSourcemapCaching failed intermittently with "error unmarshaling fields: unexpected end of JSON input". The root cause is in espoll.SearchHit.UnmarshalJSON: it calls json.Unmarshal(nil, &h.Fields) when the fields key is absent from an ES hit, and Go's json.Unmarshal(nil, ...) returns that error. This occurs when a data stream is freshly created after deletion — ES may omit fields from hits under certain timing conditions.

Motivation/summary

Two bugs fixed in systemtest/estest/search.go:

• Swapped strings.Split args in ExpectMinDocs: strings.Split(",", index) → strings.Split(index, ","). The pre-search index refresh was targeting the literal string "," instead of the actual index name, making the refresh a no-op.

• ExpectSourcemapError rewrite: replaced ExpectMinDocs (which uses espoll.NewSearchRequest with "fields": ["*"], triggering SearchHit.UnmarshalJSON) with a new searchSourcemapDocs helper that:

  • Requests only _source — not fields — avoiding the nil unmarshal path entirely
  • Uses a local struct type to parse the response, bypassing espoll.SearchHit.UnmarshalJSON
  • Constructs an espoll.SearchResult with RawSource populated (sufficient for isFetcherAvailable and assertSourcemapUpdated, which only inspect RawSource)

Checklist

• Update CHANGELOG.asciidoc
• Documentation has been updated

Related issues

Fixes #21486

• Fixes test: flaky TestSourcemapFetcher #21486

[mergify]

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

• backport-8.19 is the label to automatically backport to the 8.19 branch.
• backport-9./d is the label to automatically backport to the 9./d branch. /d is the digit.
• backport-active-all is the label that automatically backports to all active branches.
• backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

[carsonip]

@copilot how does it cause any problem if ExpectMinDocs is used in ExpectSourcemapError? Any way to handle espoll.SearchHit.UnmarshalJSON error gracefully?

[Copilot]

@copilot how does it cause any problem if ExpectMinDocs is used in ExpectSourcemapError? Any way to handle espoll.SearchHit.UnmarshalJSON error gracefully?

When ExpectMinDocs is used, it calls es.NewSearchRequest(index) which sets the request body to {"fields": ["*"]}. Elasticsearch may omit the fields key entirely from individual hits when a data stream is freshly created after deletion (a timing edge case). espoll.SearchHit.UnmarshalJSON then calls json.Unmarshal(h.RawFields, &h.Fields) where h.RawFields is nil (absent JSON field → nil json.RawMessage). Go's json.Unmarshal(nil, ...) returns "unexpected end of JSON input", which surfaces as a test failure.

Yes, the upstream fix would be a simple nil/length guard in espoll.SearchHit.UnmarshalJSON:

if len(h.RawFields) > 0 {
    if err := json.Unmarshal(h.RawFields, &h.Fields); err != nil {
        return fmt.Errorf("error unmarshaling fields: %w", err)
    }
}

That change belongs in github.com/elastic/apm-tools (a separate repo). If you'd prefer that path, I can open a PR there and then update the dependency here — that would let us revert searchSourcemapDocs and use ExpectMinDocs directly again. Otherwise, the current workaround (requesting only _source, bypassing UnmarshalJSON) is self-contained in this repo.

[earfman]

Nice catch on the root cause — the fields-absent → json.Unmarshal(nil, &h.Fields) failure in espoll.SearchHit.UnmarshalJSON is real, and the strings.Split(",", index) → strings.Split(index, ",") swap is a genuine independent fix (the refresh was a no-op before).

One thing worth flagging on searchSourcemapDocs before it lands: because it builds espoll.SearchHit{RawSource: h.Source} and never sets RawFields, it moves the nil-unmarshal downstream rather than removing it. ExpectSourcemapError returns that result, and four tests feed it into approvaltest.ApproveFields:

• systemtest/rum_test.go — TestRUMRoutingIntegration
• systemtest/sourcemap_test.go — TestRUMErrorSourcemapping, TestRUMSpanSourcemapping, TestNoMatchingSourcemap

ApproveFields reads hit.RawFields and (in approveFields) does json.Unmarshal(doc, &fields); with doc == nil that returns unexpected end of JSON input and t.Fatals. So for these four the change trades an intermittent flake for a deterministic failure: on main they pass whenever the response carries fields (exactly when ExpectMinDocs populated RawFields); after this PR RawFields is always nil, so they fail every run — and these run in the system-test CI job.

Reproduced by shaping hits the way each path builds them and calling ApproveFields:

merge-base (RawFields populated) → decodes fields, reaches the normal approval diff
pr-head    (RawFields nil)       → "unexpected end of JSON input" (t.Fatal), before any comparison

The upstream len(h.RawFields) > 0 guard you proposed above is the cleaner path precisely because it keeps ExpectMinDocs, so RawFields stays populated and ApproveFields keeps working. If you'd rather keep it in-repo, those four call sites would need ApproveEvents (which reads RawSource) or searchSourcemapDocs would need to request fields too.

— flagged via Coretexa (adversarial PR verification)