Skip to content
This repository was archived by the owner on Dec 23, 2023. It is now read-only.

Start dropping spans when the span exporter thread holds more than 10K spans references.#1893

Merged
bogdandrutu merged 5 commits intocensus-instrumentation:masterfrom
bogdandrutu:dropspans
May 13, 2019
Merged

Start dropping spans when the span exporter thread holds more than 10K spans references.#1893
bogdandrutu merged 5 commits intocensus-instrumentation:masterfrom
bogdandrutu:dropspans

Conversation

@bogdandrutu
Copy link
Copy Markdown
Contributor

Fixes #1767

public final class SpanExporterImpl extends SpanExporter {
private static final Logger logger = Logger.getLogger(ExportComponent.class.getName());
private static final DerivedLongCumulative droppedSpans =
Metrics.getMetricRegistry()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracing impl now depends on Metrics - that seems fine - I prefer not to have cyclic dependencies but, iiuc, this doesn't introduce them. Interesting issue, tho - if we split the artifacts for tracing and metrics, this won't work.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends only on the API artifact not the impl artifact.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My second point was that a service that links in a tracing impl but not a metrics impl will get unexpected results - not an issue with the current way we package artifacts but could be a problem if we ever decide to split the implementation artifacts.

Copy link
Copy Markdown
Contributor

@songy23 songy23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bogdandrutu bogdandrutu merged commit 2c844db into census-instrumentation:master May 13, 2019
@bogdandrutu bogdandrutu deleted the dropspans branch May 13, 2019 23:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory settings

5 participants