making OtlpLogExporter public

[juliuskoval]

Fixes #4971

Changes

Making OtlpLogExporter public.

Merge requirement checklist

• CONTRIBUTING guidelines followed (nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: juliuskoval / name: Julius Koval (17f4679, c3edb4d)
• ✅ login: cijothomas / name: Cijo Thomas (04961fd, 8dad02c)

[cijothomas]

Looks good.
Please add a changelog entry as well.

[codecov]

Codecov Report

Merging #4979 (04961fd) into main (637ddcf) will decrease coverage by 0.15%.
The diff coverage is n/a.

❗ Current head 04961fd differs from pull request most recent head 8dad02c. Consider uploading reports for the commit 8dad02c to get more accurate results

@@            Coverage Diff             @@
##             main    #4979      +/-   ##
==========================================
- Coverage   83.54%   83.39%   -0.15%     
==========================================
  Files         295      295              
  Lines       12336    12336              
==========================================
- Hits        10306    10288      -18     
- Misses       2030     2048      +18     

Flag	Coverage Δ
unittests	83.39% <ø> (-0.15%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
....Exporter.OpenTelemetryProtocol/OtlpLogExporter.cs	100.00% <ø> (ø)

... and 4 files with indirect coverage changes

[utpilla]

@CodeBlanch How do you feel about using a public sealed modifier instead of just public? All the other public classes are not marked sealed. Should we unmark sealed from this class as well?

[CodeBlanch]

Probably fine to make it public. The reason to seal it would be to reserve the right to make changes to it in the future by preventing people from making more derived types. But just looking at the class, I don't see a strong need for that.

If we really wanted to make it nicely extensible we should consider the use cases. The class as it is probably isn't too useful for extension. Consider something like this...

public class OtlpLogExporter
{
    public override ExportResult Export(in Batch<LogRecord> logRecordBatch)
    {
            // ...some logic omitted...
            request.AddBatch(this.sdkLimitOptions, this.ProcessResource, activityBatch);

            OnRequestSending(request); // Extension point

            if (!this.exportClient.SendExportRequest(request))
            {
                return ExportResult.Failure;
            }
            // ...some logic omitted...
    }

    // Give a nice extension point for manipulating the request before it is sent
    protected virtual void OnRequestSending(OtlpCollector.ExportLogsServiceRequest request) {}
}

Not saying we need to do this, just food for thought 😄

[vishweshbankwar]

Could be a good addition in future when we sort out the OtlpCollector.ExportLogsServiceRequest dependency. Right now the types are not exposed.