Disk buffering api implementation

[LikeTheSalad]

Creation of a default implementation for the new API that was previously defined here.

Note

It looks like there are a lot of changes, but 27 of those "files changes" are deleted files from the old implementation types and their test types. Also, a couple of file changes are just moving around files and/or adding minor changes.

New usage

The same old API behavior is kept for the new API's default implementation.

Writing

Setting up the signal storage objects as well as their toDisk exporters to be used in an OpenTelemetry instance.

// Root dir
File rootDir = new File("/some/root");

// Setting up span storage
SignalStorage.Span spanStorage = FileSpanStorage.create(new File(rootDir, "spans"));

// Setting up metric storage
SignalStorage.Metric metricStorage = FileMetricStorage.create(new File(rootDir, "metrics"));

// Setting up log storage
SignalStorage.LogRecord logStorage = FileLogRecordStorage.create(new File(rootDir, "logs"));

// Setting up span to disk exporter
SpanToDiskExporter spanToDiskExporter =
    SpanToDiskExporter.builder(spanStorage).setExporterCallback(spanCallback).build();
// Setting up metric to disk
MetricToDiskExporter metricToDiskExporter =
    MetricToDiskExporter.builder(metricStorage).setExporterCallback(metricCallback).build();
// Setting up log to disk exporter
LogRecordToDiskExporter logToDiskExporter =
    LogRecordToDiskExporter.builder(logStorage).setExporterCallback(logCallback).build();

Reading

// Example of reading an exporting spans from disk
OtlpHttpSpanExporter networkExporter;
Iterator<Collection<SpanData>> spanCollections = spanStorage.iterator();
while (spanCollections.hasNext()) {
    networkExporter.export(spanCollections.next());
}

More info in the README.md and DESIGN.md files.

[otelbot-java-contrib]

🔧 The result from spotlessApply was committed to the PR branch.

[tylerbenson]

@LikeTheSalad
In my experience, I've been involved in a lot of lengthy discussions within OpenTelemetry. I didn't feel the discussion this week was aggressive or overly negative. To be clear, I never intended to cause you grief and I'm sorry for the headache this has caused. It's not my intention to alienate a positive contributor from the community.

I see your point about Http/GrpcExporter being internal and agree it is indeed a problematic roadblock for my PR, so I'm going to close it until the SDK has a better option for how to proceed. Even the base class Marshaler is internal, so we would likely need to work with the SDK to expose some new interfaces to achieve my goal.

Meanwhile I'm going to unblock myself and copy over what I need of these classes to my internal implementation until the new API has been stabilized enough and I can see a way to reimplement it.

[breedx-splk]

(Disclaimer: I still need to give the code in this PR a proper review!)

Since I was mentioned a few times above and I was part of the discussion at the SIG meeting(s), I thought I would open an issue to help track this: #2203. The goal of that issue is to try and capture the common ground approach that I think we all agree is possible with some additional future work.

The timing was indeed unfortunate -- we basically had two competing and conflicting and disruptive changes get proposed at the same time.

@tylerbenson thanks for being understanding about this timing challenge and flexible enough to close your PR and circle back when the smoke clears.

@LikeTheSalad thank you for taking the time to write a thorough response. Same to you @bidetofevil.

[LikeTheSalad]

Thank you for understanding, @tylerbenson. It would indeed make things easier if the SDK made some of the APIs you mentioned public. In the meantime, while those are still internal, I think we should still be able to enable your use case by tweaking the internal disk buffering API in a further update so that you can reuse and repurpose a lot of it, which I can help with if needed.

Thanks for chiming in, @breedx-splk, I like the idea of keeping track of a potential new feature in that issue and taking our time to discuss all the details there.

[bidetofevil]

I think for me, it was unclear what was APIs were meant to be internal and what was meant to be external, especially given the use case Tyler was doing is explicitly about redoing some things that were considered internal, and that there was a desire to support it without needing to fork meant that we had to expose that part. If that was explicitly not wanted, then we probably started off on the wrong foot.

But Jason was right in pointing out there are conflicting change. We probably shouldn't have just merged the API changes, then found a way to build an acceptable solution on top that would work for Tyler. I basically encouraged scope creep, and I apologize for that. Lets finish this one and then figure out the other.

[tylerbenson]

@bidetofevil the main issue with my PR is that it uses (and requires users to provide) internal classes from the SDK, not anything specific to this codebase's structure. Until the API/SDK changes, I don't think what I wanted to do is reasonable for a contrib library. (I'm continuing to implement it privately, but that's just because I'm willing to accept breaking changes in the API/SDK.)

[breedx-splk]

Thanks for staying with it.

[LikeTheSalad]

Thanks for staying with it.

Cheers! Just checking in — is there anything else needed before we can move forward with these changes, @breedx-splk?

[breedx-splk]

Thanks for staying with it.

Cheers! Just checking in — is there anything else needed before we can move forward with these changes, @breedx-splk?

I don't think so, nah. Let's move this forward. Thanks again.