Add support for OTLP Exporter Protobuf over HTTP #1868

srikanthccv · 2021-05-23T06:54:54Z

Description

codeboten · 2021-06-10T15:38:39Z

@lonewolf3739 is this still work in progress?

srikanthccv · 2021-06-10T22:37:07Z

@codeboten there were some gaps b/w collector specification and collector implementation. I will check it again and get back to this.

codeboten

Thanks for picking this back up @lonewolf3739! Will review the code further later today. It would be great if you could also add an integration test here

srikanthccv · 2021-06-28T16:17:39Z

Thanks for picking this back up @lonewolf3739! Will review the code further later today. It would be great if you could also add an integration test here

Sure, block the PR when you review until I add the integration test.

srikanthccv · 2021-06-29T22:56:38Z

@codeboten Added the integration test. Please take a look.

ocelotl

Minor changes requested 👍

exporter/opentelemetry-exporter-otlp-proto-http/LICENSE

...porter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py

...tlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/encoder/__init__.py

Co-authored-by: Diego Hurtado <ocelotl@users.noreply.github.com>

…hon into otlp-http

codeboten

Nice! This PR is great, just a few comments.

...pentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/__init__.py

tests/opentelemetry-docker-tests/tests/otlpexporter/__init__.py

tests/opentelemetry-docker-tests/tests/docker-compose.yml

...porter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py

codeboten · 2021-07-01T20:47:51Z

...porter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py

+        self._shutdown = True
+
+
+def _headers_from_env() -> Optional[Dict[str, str]]:


No need to necessarily change this here, but i think we're repeating the pattern of splitting variables by , and = in a few places, might be nice to pull that code into a utility.

Agree. Probably some internal only private method in api/sdk.

+1 also what happens if the user wants to put a , or = character in their header/regex whatever. Is an escape sequence documented anywhere?

I believe it is documented in the specification.

Created an issue here #1949.

...tlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/encoder/__init__.py

…try/exporter/otlp/proto/http/trace_exporter/encoder/__init__.py

…hon into otlp-http

srikanthccv · 2021-07-03T03:54:30Z

List of public symbols

lzchen · 2021-07-13T16:26:26Z

...porter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py

+
+
+DEFAULT_COMPRESSION = Compression.NoCompression
+DEFAULT_ENDPOINT = "http://localhost:55681/v1/traces"


Where is this default defined in the specs?

Hm it's not defined yet but it should be 😅 open-telemetry/opentelemetry-specification#1816

lzchen · 2021-07-13T16:30:38Z

...porter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py

+        timeout: Optional[int] = None,
+        compression: Optional[Compression] = None,
+    ):
+        self._endpoint = endpoint or environ.get(


Any reason why some of these get their own helper functions but others do not for retrieving env vars?

No big reason except pulling this small piece of code into separate functions is kind of overkill and some values require extra processing.

lzchen · 2021-07-13T16:35:13Z

...porter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/trace_exporter/__init__.py

+            OTEL_EXPORTER_OTLP_TRACES_ENDPOINT,
+            environ.get(OTEL_EXPORTER_OTLP_ENDPOINT, DEFAULT_ENDPOINT),
+        )
+        self._certificate_file = certificate_file or environ.get(


Are we able to re-use the code in exporter mixin? Seems to be a lot of similarities. Maybe we should extract that code out and share it across all otlp exporters?

No that is not possible as of now since both are different packages. When we decided to split the packages we decided we are fine with repetitive code if it is trivial. Probably worth addressing in another PR for all otlp exporters.

lzchen · 2021-07-13T16:45:32Z

exporter/opentelemetry-exporter-otlp-proto-http/setup.cfg

+platforms = any
+license = Apache-2.0
+classifiers =
+    Development Status :: 5 - Production/Stable


Might have to have a discussion on whether or not we can mark this as stable since collector is technically not stable (even though otlp grpc is stable). Will talk about it in the SIG.

We didn't get a chance to talk about this in the SIG but I agree w/ @lzchen, we should mark hold off marking this as stable until the port is confirmed. We can then merge this as beta until then.

codeboten · 2021-07-16T16:59:56Z

...opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/version.py

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+__version__ = "1.4.0.dev0"


I suggest we mark this as 0.23.dev0 until the port discussion is sorted in the spec open-telemetry/opentelemetry-specification#1816

Issue on collector repo open-telemetry/opentelemetry-collector#3617. I think it will take some to get the port fixed. I will change this to 0.23.dev0.

…hon into otlp-http

srikanthccv · 2021-07-18T14:14:38Z

Marked as 0.23.dev0 and changed status to beta.

codeboten

Nice! Thanks for addressing my comments

codeboten · 2021-07-19T15:52:42Z

@lzchen @aabmass please approve if your comments have been addressed.

...opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/version.py

codeboten · 2021-07-26T15:34:42Z

@lonewolf3739 please take a look at the failing test and we can get this merged!

srikanthccv · 2021-07-26T19:10:46Z

@codeboten will update this once #1985 gets merged.

…hon into otlp-http

srikanthccv · 2021-07-29T07:24:18Z

@codeboten All tests are passing now.

codeboten · 2021-07-29T17:34:27Z

@lonewolf3739 nice thank you!

srikanthccv added 3 commits May 23, 2021 12:13

WIP: OTLP Exporter Proto over HTPP

fe42a03

Add class var for retry timeout

2428318

Update default endpoint

8007517

srikanthccv added 2 commits June 26, 2021 06:09

Resolve merge conflicts

82bd4ad

fix version failures

391af28

srikanthccv requested review from codeboten and lzchen June 26, 2021 00:52

srikanthccv changed the title ~~[WIP] Add support for OTLP Exporter Protobuf over HTTP~~ Add support for OTLP Exporter Protobuf over HTTP Jun 26, 2021

srikanthccv marked this pull request as ready for review June 26, 2021 00:53

srikanthccv requested a review from a team as a code owner June 26, 2021 00:53

codeboten reviewed Jun 28, 2021

View reviewed changes

srikanthccv added 2 commits June 30, 2021 04:21

Add CHANGELOG entry

d171369

Add functional test for OTLP/HTTP exporter

176d60a

ocelotl requested changes Jul 1, 2021

View reviewed changes

srikanthccv and others added 4 commits July 1, 2021 20:45

Update exporter/opentelemetry-exporter-otlp-proto-http/LICENSE

c252c38

Co-authored-by: Diego Hurtado <ocelotl@users.noreply.github.com>

Add review suggestion

92e005f

Merge branch 'otlp-http' of github.com:lonewolf3739/opentelemetry-pyt…

834d7b6

…hon into otlp-http

Merge branch 'main' into otlp-http

3766f84

ocelotl approved these changes Jul 1, 2021

View reviewed changes

codeboten reviewed Jul 1, 2021

View reviewed changes

srikanthccv added 4 commits July 3, 2021 02:42

Make proto encoder private

f66811a

Update exporter/opentelemetry-exporter-otlp-proto-http/src/openteleme…

8c283f9

…try/exporter/otlp/proto/http/trace_exporter/encoder/__init__.py

Merge branch 'otlp-http' of github.com:lonewolf3739/opentelemetry-pyt…

aaccd7f

…hon into otlp-http

Merge branch 'main' into otlp-http

2a8833e

srikanthccv added the Approve Public API check This label shows that the public symbols added or changed in a PR are strictly necessary label Jul 7, 2021

lzchen reviewed Jul 13, 2021

View reviewed changes

codeboten reviewed Jul 16, 2021

View reviewed changes

srikanthccv added 4 commits July 18, 2021 19:00

Update version and status to beta

b5692ff

Merge branch 'main' into otlp-http

268fe36

Remove beta proto http from otlp-combined

88f84b9

Merge branch 'otlp-http' of github.com:lonewolf3739/opentelemetry-pyt…

17215e8

…hon into otlp-http

codeboten approved these changes Jul 19, 2021

View reviewed changes

codeboten requested review from lzchen and aabmass July 19, 2021 15:52

Merge branch 'main' into otlp-http

2bb3064

lzchen reviewed Jul 19, 2021

View reviewed changes

...opentelemetry-exporter-otlp-proto-http/src/opentelemetry/exporter/otlp/proto/http/version.py Outdated Show resolved Hide resolved

lzchen approved these changes Jul 19, 2021

View reviewed changes

ocelotl and others added 2 commits July 22, 2021 10:22

Merge branch 'main' into otlp-http

3562bbc

Merge branch 'main' into otlp-http

7b350e7

srikanthccv added 3 commits July 28, 2021 04:01

Merge branch 'main' into otlp-http

1d70df8

Bump version

17ab52e

Merge branch 'otlp-http' of github.com:lonewolf3739/opentelemetry-pyt…

d009590

…hon into otlp-http

codeboten merged commit 9d55e64 into open-telemetry:main Jul 29, 2021

MikeGoldsmith mentioned this pull request Aug 17, 2021

Project is being sunset honeycombio/opentelemetry-exporter-python#36

Closed

zawi99 mentioned this pull request Aug 25, 2021

Support of opentelemetry v1.4 for django and celery instrumentations open-telemetry/opentelemetry-python-contrib#626

Closed

srikanthccv deleted the otlp-http branch September 24, 2021 08:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for OTLP Exporter Protobuf over HTTP #1868

Add support for OTLP Exporter Protobuf over HTTP #1868

srikanthccv commented May 23, 2021 •

edited

codeboten commented Jun 10, 2021

srikanthccv commented Jun 10, 2021

codeboten left a comment

srikanthccv commented Jun 28, 2021

srikanthccv commented Jun 29, 2021

ocelotl left a comment

codeboten left a comment

codeboten Jul 1, 2021

srikanthccv Jul 1, 2021

aabmass Jul 12, 2021

srikanthccv Jul 12, 2021

srikanthccv Jul 13, 2021

srikanthccv commented Jul 3, 2021

lzchen Jul 13, 2021

NathanielRN Jul 19, 2021

lzchen Jul 13, 2021

srikanthccv Jul 13, 2021

lzchen Jul 13, 2021

srikanthccv Jul 13, 2021

lzchen Jul 13, 2021

codeboten Jul 16, 2021 •

edited

codeboten Jul 16, 2021

srikanthccv Jul 16, 2021

srikanthccv commented Jul 18, 2021

codeboten left a comment

codeboten commented Jul 19, 2021

codeboten commented Jul 26, 2021

srikanthccv commented Jul 26, 2021

srikanthccv commented Jul 29, 2021

codeboten commented Jul 29, 2021

		self._shutdown = True


		def _headers_from_env() -> Optional[Dict[str, str]]:



		DEFAULT_COMPRESSION = Compression.NoCompression
		DEFAULT_ENDPOINT = "http://localhost:55681/v1/traces"

Add support for OTLP Exporter Protobuf over HTTP #1868

Add support for OTLP Exporter Protobuf over HTTP #1868

Conversation

srikanthccv commented May 23, 2021 • edited

Description

codeboten commented Jun 10, 2021

srikanthccv commented Jun 10, 2021

codeboten left a comment

Choose a reason for hiding this comment

srikanthccv commented Jun 28, 2021

srikanthccv commented Jun 29, 2021

ocelotl left a comment

Choose a reason for hiding this comment

codeboten left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srikanthccv commented Jul 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeboten Jul 16, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srikanthccv commented Jul 18, 2021

codeboten left a comment

Choose a reason for hiding this comment

codeboten commented Jul 19, 2021

codeboten commented Jul 26, 2021

srikanthccv commented Jul 26, 2021

srikanthccv commented Jul 29, 2021

codeboten commented Jul 29, 2021

srikanthccv commented May 23, 2021 •

edited

codeboten Jul 16, 2021 •

edited