Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ForceFlush missing from Exporter spec #1454

Closed
Oberon00 opened this issue Feb 20, 2021 · 2 comments · Fixed by #1467
Closed

ForceFlush missing from Exporter spec #1454

Oberon00 opened this issue Feb 20, 2021 · 2 comments · Fixed by #1467
Labels
area:sdk Related to the SDK bug Something isn't working spec:trace Related to the specification/trace directory

Comments

@Oberon00
Copy link
Member

We currently have ForceFlush only specified in the SpanProcessor. #1452 adds the missing "helper" method to call this to the SdkTracerProvider. But we are also missing ForceFlush in the spec for exporters (some languages, e.g. Java, have it implemented already (https://github.com/open-telemetry/opentelemetry-java/blob/v0.17.1/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/export/SpanExporter.java#L69-L77).

In fact this was proposed in the original issue to add flush/forceFlush (#351) which was titled "Add flush API to processor/exporter". However it seems to have been glanced over in #370 (in turn, then some SIGs seemed to have glanced over the fact that it was never spec'd and implemented it anyway 😉).

In some languages, adding a new method to an interface might be a breaking change for existing implementers. However, there are simple workarounds for that such as adding Flush to an new interface like FlushableExporter extends Exporter. I think it is OK to leave this implementation detail to the language SIGs.

@Oberon00 Oberon00 added spec:trace Related to the specification/trace directory area:sdk Related to the SDK bug Something isn't working labels Feb 20, 2021
@bogdandrutu
Copy link
Member

I think we need to add some text about this, languages should choose what is best for them that allows to extent the API (java8 has default implementation, go can rely o duci typing, etc.) that will allow them to extend the interfaces. And we should not consider breaking change adding a new functionality on an interface. Will file a separate issue.

@Oberon00
Copy link
Member Author

@bogdandrutu I created #1457 for the interface versioning issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sdk Related to the SDK bug Something isn't working spec:trace Related to the specification/trace directory
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants