span (experimental): Compress short exit spans #1134

marclop · 2021-10-05T06:02:37Z

Description

Implements the span compression algorithm described in elastic/apm#432.
The implementation pretty close to what the spec describes, with a few
modifications due to the differences in the Go agent implementation. The
span compression is experimental and is disabled by default with the
default thresholds for the different compression strategies set:

ELASTIC_APM_SPAN_COMPRESSION_ENABLED=false
ELASTIC_APM_SPAN_COMPRESSION_EXACT_MATCH_MAX_DURATION=50ms
ELASTIC_APM_SPAN_COMPRESSION_SAME_KIND_MAX_DURATION=5ms

The implementation uses a new field (cache *Span) in TransactionData
and SpanData which is used to cache compressed or compression eligible
spans in that field. An additional composite field has also been added
to the SpanData and TransactionData.

The algorithm states that any exit span that is lower or equal duration
than the set ELASTIC_APM_SPAN_COMPRESSION_EXACT_MATCH_MAX_DURATION or
ELASTIC_APM_SPAN_COMPRESSION_SAME_KIND_MAX_DURATION with a destination
service set is compressable into a composite span using the appropriate
strategy. Compressable spans need to be "exit" and siblings which have
not propagated their context downstream. Sibling spans are spans that
share the same parent span (or transaction).

Spans can be compressed with same_kind or exact_match as a strategy,
and their sum is the aggregated duration of of all
When a compressed span has been compressed into a composite using the
same_kind strategy, its name is mutated to Calls to <span.Type>.

Spans will be compressed into a composite only when the siblings are
consecutive and any compression attempt that doesn't meet that,
will cause the cache to be evicted.

Additionally, two helper functions have been added under the apmtest:
package: WriteTraceTable and WriteTraceWaterfall, which help with
understanding why a test is failing if they are and debugging those.

TODO

Changelog
Docs

Related issues

Closes #972

Signed-off-by: Marc Lopez Rubio <[email protected]>

apmmachine · 2021-10-05T06:07:27Z

💚 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: 2021-10-13T09:52:14.480+0000
Duration: 35 min 5 sec
Commit: c9d42c2

Test stats 🧪

Test	Results
Failed	0
Passed	11425
Skipped	268
Total	11693

🤖 GitHub comments

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

/test : Re-trigger the build.
run benchmark tests : Run the benchmark test.

Signed-off-by: Marc Lopez Rubio <[email protected]>

axw

The approach looks good. I haven't looked at all of the finer details, just left a few initial comments. I'm not entirely convinced by the locking, but again I haven't pored over all the details.

span.go

span_compressed.go

span.go

Signed-off-by: Marc Lopez Rubio <[email protected]>

Removes the `spanBuffer`, adding a new field to `Transaction` and `Span` instead and handling the caching directly on that field. `evictCache` is used to flush the `TransactionData.cache` and `SpanData.cache` Also simplifies a few functions and uses a `strings.Builder` with the total capacity set beforehand to update the `span.Name` since that seems to be the fastest and most efficient way to concatenate the two strings, the allocation is unavoidable. Signed-off-by: Marc Lopez Rubio <[email protected]>

Signed-off-by: Marc Lopez Rubio <[email protected]>

marclop · 2021-10-07T07:28:46Z

I'll tidy the tests up a little bit before setting the PR for review, but I believe that the implementation should be complete. I'll update the PR description with the current implementation's details as well.

Signed-off-by: Marc Lopez Rubio <[email protected]>

transaction.go

span_compressed.go

Signed-off-by: Marc Lopez Rubio <[email protected]>

Avoid `s.enqueue` from `s.attemptCompress` function and instead, return the evicted span from the cache and use the `span.End()` function. A new bool field has been introduced (in `Span.fromCache`), set to `true` when the a span has been evicted from the cache. The purpose of this flag is to avoid reporting the span timers since they've already been reported on each `span.End`, and to prevent the `span.End()` from going into an infinite loop, trying to compress a span that has already been evicted. Also, updates the mutex operations to make all operations concurrently safe. Signed-off-by: Marc Lopez Rubio <[email protected]>

marclop · 2021-10-12T06:32:15Z

@axw I should have addressed all your concerns in b86661f. Thank you for the help.

config_test.go

docs/configuration.asciidoc

span.go

span_compressed.go

Adds a new `s.compressOrEvictCache`, and simplifies `s.attemptCompress`. The new method compresses or evicts the span into / from the cache. Also modifies the `s.evict()` to set the cached span compression options to `false` so the span doesn't go through the compression flow again and causes infinite loop. Last, it splits the compression and enqueuing portion of `s.End()` into a new `s.end()` method which is called only for spans that have been evicted from the cache. Signed-off-by: Marc Lopez Rubio <[email protected]>

Signed-off-by: Marc Lopez Rubio <[email protected]>

span_compressed.go

Signed-off-by: Marc Lopez Rubio <[email protected]>

axw

LGTM. Nice work!

marclop added 6 commits October 5, 2021 13:32

Add config options

b65eb87

Signed-off-by: Marc Lopez Rubio <[email protected]>

Add models

bf8b1fa

Signed-off-by: Marc Lopez Rubio <[email protected]>

Add span compression logic

00730c4

Signed-off-by: Marc Lopez Rubio <[email protected]>

Add tests and test debug helpers

47c5f6c

Signed-off-by: Marc Lopez Rubio <[email protected]>

Add an additional test case

49acc9a

Signed-off-by: Marc Lopez Rubio <[email protected]>

apmtest: Rename PrintTraces to WriteTraceTable

1085f4b

Signed-off-by: Marc Lopez Rubio <[email protected]>

marclop added enhancement New feature or request v7.16.0 labels Oct 5, 2021

marclop self-assigned this Oct 5, 2021

github-actions bot added the agent-go label Oct 5, 2021

Update tests

30eed35

Signed-off-by: Marc Lopez Rubio <[email protected]>

marclop added the [zube]: In Progress label Oct 5, 2021

Don't use time.Duration.Milliseconds()

6bc3358

Signed-off-by: Marc Lopez Rubio <[email protected]>

axw reviewed Oct 5, 2021

View reviewed changes

marclop added 7 commits October 6, 2021 07:51

Address review comments

f0bee15

Signed-off-by: Marc Lopez Rubio <[email protected]>

Move round to apmmath.Round()

354ef44

Signed-off-by: Marc Lopez Rubio <[email protected]>

Update unit tests

8c05116

Signed-off-by: Marc Lopez Rubio <[email protected]>

Merge 'upstream/master'

e1ce81a

Merge 'upstream/master'

a726004

Abstract strings.Builder under apmstrings

d5aee2e

Signed-off-by: Marc Lopez Rubio <[email protected]>

marclop added 2 commits October 11, 2021 09:04

Tidy up tests

3a08a51

Signed-off-by: Marc Lopez Rubio <[email protected]>

Add changelog and document config options

353562b

Signed-off-by: Marc Lopez Rubio <[email protected]>

marclop added [zube]: In Review and removed [zube]: In Progress labels Oct 11, 2021

marclop marked this pull request as ready for review October 11, 2021 02:37

marclop requested a review from a team October 11, 2021 02:40

Add dynamic badges to config options

2341ff0

Signed-off-by: Marc Lopez Rubio <[email protected]>

axw reviewed Oct 11, 2021

View reviewed changes

transaction.go Outdated Show resolved Hide resolved

span_compressed.go Outdated Show resolved Hide resolved

span_compressed.go Outdated Show resolved Hide resolved

span_compressed.go Outdated Show resolved Hide resolved

Properly set up dynamic agent config

d2de091

Signed-off-by: Marc Lopez Rubio <[email protected]>

marclop added [zube]: In Progress and removed [zube]: In Review labels Oct 12, 2021

marclop added [zube]: In Review and removed [zube]: In Progress labels Oct 12, 2021

axw requested changes Oct 13, 2021

View reviewed changes

marclop added 2 commits October 13, 2021 15:30

Add amended tests

0b0b6f2

Signed-off-by: Marc Lopez Rubio <[email protected]>

axw reviewed Oct 13, 2021

View reviewed changes

span_compressed.go Outdated Show resolved Hide resolved

span_compressed.go Show resolved Hide resolved

marclop added 2 commits October 13, 2021 17:50

Only compress spans when they are siblings

82bcc1b

Signed-off-by: Marc Lopez Rubio <[email protected]>

Update comment

c9d42c2

Signed-off-by: Marc Lopez Rubio <[email protected]>

axw approved these changes Oct 13, 2021

View reviewed changes

marclop merged commit 27abca2 into elastic:master Oct 13, 2021

marclop deleted the f/add-span-compression-feature branch October 13, 2021 10:37

zube bot added [zube]: Done and removed [zube]: In Review labels Oct 13, 2021

bmorelli25 mentioned this pull request Nov 23, 2021

[APM] Update hard-coded links to Go Agent elastic/observability-docs#1285

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

span (experimental): Compress short exit spans #1134

span (experimental): Compress short exit spans #1134

marclop commented Oct 5, 2021 •

edited

Loading

apmmachine commented Oct 5, 2021 •

edited

Loading

Build stats

Test stats 🧪

axw left a comment

marclop commented Oct 7, 2021

marclop commented Oct 12, 2021

axw left a comment

span (experimental): Compress short exit spans #1134

span (experimental): Compress short exit spans #1134

Conversation

marclop commented Oct 5, 2021 • edited Loading

Description

TODO

Related issues

apmmachine commented Oct 5, 2021 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

axw left a comment

Choose a reason for hiding this comment

marclop commented Oct 7, 2021

marclop commented Oct 12, 2021

axw left a comment

Choose a reason for hiding this comment

marclop commented Oct 5, 2021 •

edited

Loading

apmmachine commented Oct 5, 2021 •

edited

Loading