Deprecate otlp_proto_grpc and otlp_proto_http in auto-instrumentation #1250

ronyis · 2022-08-29T17:01:54Z

Description

Fixes open-telemetry/opentelemetry-python#2586

This PR depends on SDK-2893 , to support setting OTLP transport protocol with env vars, instead of using otlp_proto_grpc / otlp_proto_http.

This method should be preferred, as it is defined in the specifications (it is also how it works in other OTel libs).

This PR fixes open-telemetry/opentelemetry-python#2586, by allowing the user to configure the protocol using OTEL_EXPORTER_OTLP_PROTOCOL without making a conflict with the current default otlp_proto_grpc setting. It also updates documentation on how to set the protocol.

Type of change

Please delete options that are not relevant.

New feature (non-breaking change which adds functionality)
This change requires a documentation update

How Has This Been Tested?

opentelemetry-distro/tests/test_distro.py

Does This PR Require a Core Repo Change?

Yes. - Link to PR: Adding support for setting OTLP exporter protocol by env vars opentelemetry-python#2893

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

Followed the style guidelines of this project
Changelogs have been updated (in core package only)
Unit tests have been added
Documentation has been updated

ronyis · 2022-08-29T18:23:04Z

opentelemetry-instrumentation/README.rst

@@ -22,7 +22,7 @@ This package provides a couple of commands that help automatically instruments a
    For more info about ``opentelemetry-distro`` check `here <https://opentelemetry-python.readthedocs.io/en/latest/examples/distro/README.html>`__
    ::

-        pip install opentelemetry-distro[otlp]
+        pip install opentelemetry-distro


Looks like this is how it is installed (also specified in https://github.com/open-telemetry/opentelemetry-python-contrib/tree/main/opentelemetry-distro#installation)

That's different. The auto instrumentation requires it this way otherwise without any exporter users won't see any telemetry.

ronyis · 2022-08-29T18:25:38Z

opentelemetry-instrumentation/README.rst

+    - otlp_proto_grpc (`deprecated`)
+    - otlp_proto_http (`deprecated`)


I think we should not recommend using this, as the defined way in OTel to configure the protocol is with the other env vars, and to avoid conflicts.

In future versions, we may also remove these options (though it's a breaking change).

ronyis · 2022-08-29T18:27:12Z

opentelemetry-instrumentation/README.rst


-``otlp`` is an alias for ``otlp_proto_grpc``.
+Note: The default transport protocol for ``otlp`` is gRPC.
+HTTP is currently supported for traces only, and should be set using ``OTEL_EXPORTER_OTLP_TRACES_PROTOCOL=http/protobuf``


I suggest not using the more general way of OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf, since it's still not supported for metrics & logs.

ronyis · 2022-09-07T09:24:53Z

open-telemetry/opentelemetry-python#2893 is already merged, this PR awaits review

srikanthccv · 2022-09-09T20:02:15Z

opentelemetry-instrumentation/README.rst

@@ -22,7 +22,7 @@ This package provides a couple of commands that help automatically instruments a
    For more info about ``opentelemetry-distro`` check `here <https://opentelemetry-python.readthedocs.io/en/latest/examples/distro/README.html>`__
    ::

-        pip install opentelemetry-distro[otlp]
+        pip install opentelemetry-distro


That's different. The auto instrumentation requires it this way otherwise without any exporter users won't see any telemetry.

opentelemetry-distro/src/opentelemetry/distro/__init__.py

…pentelemetry-python-contrib into otlp-protocol-env-vars

opentelemetry-distro/tests/test_distro.py

Co-authored-by: Srikanth Chekuri <srikanth.chekuri92@gmail.com>

ronyis · 2022-09-12T09:47:46Z

@srikanthccv can you please add a skip-changelog label?
(I think the changelog added in the SDK is sufficient since the logic is there)

srikanthccv · 2022-09-12T10:12:43Z

Please add the CHANGELOG entry. It's needed here also for visibility for the end users who use the instrumentation packages. The SDK PR makes it complaint with spec but this one says they are deprecated.

ronyis · 2022-09-20T16:43:01Z

Is there anything missing to merge this PR? (@srikanthccv)

srikanthccv · 2022-09-20T17:35:02Z

Your branch is out of date with the main branch. You didn't enable maintainers to push commits to your branch so we can't update it. This won't be merged until it is updated with the main changes at the time of merge.

srikanthccv · 2022-10-01T10:41:33Z

@ronyis update/rebase the branch if you would like to get this merged.

srikanthccv · 2022-10-11T13:00:11Z

CHANGELOG.md

@@ -40,6 +40,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

 ### Fixed

+- `opentelemetry-distro` Deprecate `otlp_proto_grpc` and `otlp_proto_http` in favor of using
+  `OTEL_EXPORTER_OTLP_TRACES_PROTOCOL` as according to specifications
+  ([#1250](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1250))


Move this to Unreleased section

ocelotl · 2022-10-12T12:08:43Z

@srikanthccv please remove your approval if there are still pending changes from the submitter to prevent accidental merging.

srikanthccv · 2022-10-12T13:19:23Z

CHANGELOG.md

@@ -40,6 +40,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

 ### Fixed

+- `opentelemetry-distro` Deprecate `otlp_proto_grpc` and `otlp_proto_http` in favor of using
+  `OTEL_EXPORTER_OTLP_TRACES_PROTOCOL` as according to specifications
+  ([#1250](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1250))


ronyis · 2022-10-18T13:52:56Z

@srikanthccv Should be fixed now (sorry for the delay)

srikanthccv · 2022-10-21T15:20:24Z

Please update with the main

…open-telemetry#1250)

MasaNahti · 2023-01-20T10:32:43Z

This deprecation has not been documented in Python automatic instumentation configuration docs. It took me awhile to figure it out, since I assumed the docs as the best source of truth. There are also plenty other mistakes there, but that's another topic.

ronyis added 2 commits August 29, 2022 19:36

Deprecate otlp_proto_grpc and otlp_proto_http in auto-instrumentation

7f6a041

tests

da269f5

ronyis mentioned this pull request Aug 29, 2022

Adding support for setting OTLP exporter protocol by env vars open-telemetry/opentelemetry-python#2893

Merged

5 tasks

ronyis commented Aug 29, 2022

View reviewed changes

ronyis marked this pull request as ready for review August 29, 2022 18:27

ronyis requested a review from a team August 29, 2022 18:27

lint

922b349

galbash approved these changes Aug 30, 2022

View reviewed changes

Merge branch 'main' into otlp-protocol-env-vars

be7dce8

srikanthccv reviewed Sep 9, 2022

View reviewed changes

ronyis added 3 commits September 11, 2022 19:30

CR fix

f96e768

Merge branch 'otlp-protocol-env-vars' of https://github.com/epsagon/o…

6cbf14d

…pentelemetry-python-contrib into otlp-protocol-env-vars

Merge branch 'main' into otlp-protocol-env-vars

ea4e189

srikanthccv reviewed Sep 11, 2022

View reviewed changes

opentelemetry-distro/tests/test_distro.py Outdated Show resolved Hide resolved

Update opentelemetry-distro/tests/test_distro.py

612c86d

Co-authored-by: Srikanth Chekuri <srikanth.chekuri92@gmail.com>

srikanthccv approved these changes Sep 11, 2022

View reviewed changes

lint

67b07e9

ronyis added 2 commits September 12, 2022 19:43

Updating changelog

7c9b724

Merge branch 'main' into otlp-protocol-env-vars

626289f

Merge branch 'main' into otlp-protocol-env-vars

34f76ac

srikanthccv reviewed Oct 11, 2022

View reviewed changes

srikanthccv suggested changes Oct 12, 2022

View reviewed changes

ronyis added 2 commits October 18, 2022 16:48

Merge branch 'main' into otlp-protocol-env-vars

32c3b31

Update CHANGELOG.md

1b89f29

ronyis added 2 commits October 27, 2022 13:16

Merge branch 'main' into otlp-protocol-env-vars

2aab7ff

Fixing CHANGELOG.md

448dff7

srikanthccv approved these changes Oct 27, 2022

View reviewed changes

srikanthccv merged commit 83f37ed into open-telemetry:main Oct 27, 2022

ronyis deleted the otlp-protocol-env-vars branch October 27, 2022 12:42

saartochner-lumigo pushed a commit to lumigo-io/opentelemetry-python-contrib that referenced this pull request Nov 13, 2022

Deprecate otlp_proto_grpc and otlp_proto_http in auto-instrumentation (…

883004d

…open-telemetry#1250)

saartochner-lumigo pushed a commit to lumigo-io/opentelemetry-python-contrib that referenced this pull request Nov 13, 2022

Deprecate otlp_proto_grpc and otlp_proto_http in auto-instrumentation (…

c2a559e

…open-telemetry#1250)

ronyis mentioned this pull request Feb 5, 2023

Update Python agent-config open-telemetry/opentelemetry.io#2272

Merged

sy-be mentioned this pull request Feb 28, 2024

Inject SDK & environment variables but do not autoinstrument with init container open-telemetry/opentelemetry-operator#2681

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate otlp_proto_grpc and otlp_proto_http in auto-instrumentation #1250

Deprecate otlp_proto_grpc and otlp_proto_http in auto-instrumentation #1250

ronyis commented Aug 29, 2022 •

edited

Loading

ronyis Aug 29, 2022

srikanthccv Sep 9, 2022

ronyis Aug 29, 2022

ronyis Aug 29, 2022

ronyis commented Sep 7, 2022

srikanthccv Sep 9, 2022

ronyis commented Sep 12, 2022

srikanthccv commented Sep 12, 2022

ronyis commented Sep 20, 2022

srikanthccv commented Sep 20, 2022

srikanthccv commented Oct 1, 2022

srikanthccv Oct 11, 2022

srikanthccv Oct 12, 2022

ocelotl commented Oct 12, 2022

srikanthccv Oct 12, 2022

ronyis commented Oct 18, 2022

srikanthccv commented Oct 21, 2022

MasaNahti commented Jan 20, 2023

		- otlp_proto_grpc (`deprecated`)
		- otlp_proto_http (`deprecated`)

Deprecate otlp_proto_grpc and otlp_proto_http in auto-instrumentation #1250

Deprecate otlp_proto_grpc and otlp_proto_http in auto-instrumentation #1250

Conversation

ronyis commented Aug 29, 2022 • edited Loading

Description

Type of change

How Has This Been Tested?

Does This PR Require a Core Repo Change?

Checklist:

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

ronyis commented Sep 7, 2022

Choose a reason for hiding this comment

ronyis commented Sep 12, 2022

srikanthccv commented Sep 12, 2022

ronyis commented Sep 20, 2022

srikanthccv commented Sep 20, 2022

srikanthccv commented Oct 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocelotl commented Oct 12, 2022

Choose a reason for hiding this comment

ronyis commented Oct 18, 2022

srikanthccv commented Oct 21, 2022

MasaNahti commented Jan 20, 2023

ronyis commented Aug 29, 2022 •

edited

Loading