Abstract out OTLP http exporter logic

[lorenzoronzani]

Description

This PR contains the refactoring required inside the http_exporter_logic component. The idea is to remove the duplicated code abstracting out inside a common section

Fixes #2990

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactor (functionalities are the same, interfaces also but the logic is moved inside codebase)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• Executed component specific unit tests using the following command uv run tox -e py312-test-opentelemetry-exporter-otlp-proto-http

Does This PR Require a Contrib Repo Change?

• Yes. - Link to PR:
• No. No, it's just an internal refactoring

Checklist:

• Followed the style guidelines of this project
• Changelogs have been updated
• Unit tests have been added/updated
• Documentation has been updated, this is a pure internal refactor, not interfaces update so I don't think that a documentation update it's required

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: lorenzoronzani / name: Lorenzo Ronzani (18cd52b, 089a7c7, 09eec1d, 0ae61f2, 0d9175d, 1eeaab5, 250d7b9, 29c7840, 2aa0053, 3380215, 39723c8, 41d324e, 4405d3e, 44a9b47, 510b7dd, 6cdbc17, 7dec4f5, 88fcdcb, 8c5497e, 963ab66, a19418f, bc8fb6f, be16153, c80b870, cb3c11c, d557b6d, e21ee19)

[lorenzoronzani]

/easycla

[herin049]

I would recommend reconsidering your approach here. I'm generally not a huge fan of having functions with a huge number of arguments. I'd generally be more in favor of creating a common OTLPHttpClient, as is done in this PR

[lorenzoronzani]

I would recommend reconsidering your approach here. I'm generally not a huge fan of having functions with a huge number of arguments. I'd generally be more in favor of creating a common OTLPHttpClient, as is done in this PR

I was thinking the same but looking around I thought that the implemented method was the one expected. I can convert with a common client class.

Thanks for the review

[lorenzoronzani]

I moved these 3 public constants:

DEFAULT_COMPRESSION
DEFAULT_ENDPOINT
DEFAULT_TIMEOUT

from each exporter (Log, Metric, Trace) into the common one but this uv run tox -e public-symbols-check check now it's failing.

Q: What is the right process in this cases?

[DylanRussell]

can this Enum be part of _SignalConfig and returned directly from OTLPHttpClient

[lorenzoronzani]

Could be but I don't like to much that the parent needs to know children. This because you need to specify a list of types but what about in the future I want to add a new type of exporter? I should edit also the parent class.

What do you think?

[aabmass]

I would recommend reconsidering your approach here. I'm generally not a huge fan of having functions with a huge number of arguments. I'd generally be more in favor of creating a common OTLPHttpClient, as is done in this PR

@herin049 @lorenzoronzani I want to make sure we're not duplicating effort. It looks like you closed #5051, was that in favor of this PR?

[herin049]

I would recommend reconsidering your approach here. I'm generally not a huge fan of having functions with a huge number of arguments. I'd generally be more in favor of creating a common OTLPHttpClient, as is done in this PR

@herin049 @lorenzoronzani I want to make sure we're not duplicating effort. It looks like you closed #5051, was that in favor of this PR?

@aabmass I actually closed out that PR to switch to #5194, given that this sounds like the direction that the SIG wants to go. It'd be better if this functionality is broken off into a separate package in order to support the work in #1003. I would suggest we close this PR or mark it as draft for now and see if we need to re-open it later.

[herin049]

I would recommend reconsidering your approach here. I'm generally not a huge fan of having functions with a huge number of arguments. I'd generally be more in favor of creating a common OTLPHttpClient, as is done in this PR

@herin049 @lorenzoronzani I want to make sure we're not duplicating effort. It looks like you closed #5051, was that in favor of this PR?

Assuming we're OK with the approach in #5194, then it will be a pre-requisite to implementing the final OTLP JSON exporter package, and can also be used to simultaneously close #2990.

[lorenzoronzani]

I would recommend reconsidering your approach here. I'm generally not a huge fan of having functions with a huge number of arguments. I'd generally be more in favor of creating a common OTLPHttpClient, as is done in this PR

@herin049 @lorenzoronzani I want to make sure we're not duplicating effort. It looks like you closed #5051, was that in favor of this PR?

Assuming we're OK with the approach in #5194, then it will be a pre-requisite to implementing the final OTLP JSON exporter package, and can also be used to simultaneously close #2990.

@herin049 @aabmass thank for the review.
Please let me know if I can help in some ways

[emdneto]

Thanks for the PR!

Just a heads-up: we no longer update CHANGELOG.md directly. The changelog is now generated from changelog fragments using Towncrier.

Please add the appropriate changelog fragment for this change instead of editing CHANGELOG.md manually. You can find the instructions and expected format in CONTRIBUTING.md.

[github-actions]

This PR has been automatically marked as stale because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 days of this comment.
If you're still working on this, please add a comment or push new commits.

[github-actions]

This PR has been closed due to inactivity. Please reopen if you would like to continue working on it.