Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document some quirks with Python otel operator based auto-instrumentation #5338

Merged
merged 16 commits into from
Oct 15, 2024

Conversation

xrmx
Copy link
Contributor

@xrmx xrmx commented Oct 7, 2024

Fix #5243

@xrmx xrmx requested a review from a team as a code owner October 7, 2024 14:45
@opentelemetrybot opentelemetrybot requested a review from a team October 7, 2024 14:45
@opentelemetrybot opentelemetrybot requested a review from a team October 7, 2024 14:51
Copy link
Member

@theletterf theletterf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions.

content/en/docs/kubernetes/operator/automatic.md Outdated Show resolved Hide resolved
content/en/docs/zero-code/python/operator.md Outdated Show resolved Hide resolved
content/en/docs/zero-code/python/operator.md Outdated Show resolved Hide resolved
@xrmx xrmx requested a review from theletterf October 8, 2024 07:26
@svrnm
Copy link
Member

svrnm commented Oct 8, 2024

/fix:all

@opentelemetrybot
Copy link
Collaborator

You triggered fix:all action run at https://github.com/open-telemetry/opentelemetry.io/actions/runs/11237644963

@opentelemetrybot
Copy link
Collaborator

fix:all failed or was cancelled. For details, see https://github.com/open-telemetry/opentelemetry.io/actions/runs/11237644963.

@svrnm
Copy link
Member

svrnm commented Oct 9, 2024

/fix:all

@opentelemetrybot
Copy link
Collaborator

You triggered fix:all action run at https://github.com/open-telemetry/opentelemetry.io/actions/runs/11254563117

@opentelemetrybot
Copy link
Collaborator

fix:all failed or was cancelled. For details, see https://github.com/open-telemetry/opentelemetry.io/actions/runs/11254563117.

@xrmx
Copy link
Contributor Author

xrmx commented Oct 9, 2024

It's openai.com returning 403, maybe they started blocking connections from github? Or some stale cache?

@svrnm
Copy link
Member

svrnm commented Oct 9, 2024

It's openai.com returning 403, maybe they started blocking connections from github? Or some stale cache?

This was introduced with merging the semconv update, rebasing the PR should fix it

@opentelemetrybot opentelemetrybot requested a review from a team October 9, 2024 13:53
@xrmx
Copy link
Contributor Author

xrmx commented Oct 9, 2024

@svrnm thanks!

@svrnm
Copy link
Member

svrnm commented Oct 11, 2024

@theletterf can you take a look if your change requestd have been applied as suggested and if so approve? thx

Copy link
Contributor

@tiffany76 tiffany76 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a few copy edit changes. Thanks!

content/en/docs/kubernetes/operator/automatic.md Outdated Show resolved Hide resolved
content/en/docs/kubernetes/operator/automatic.md Outdated Show resolved Hide resolved
content/en/docs/zero-code/python/operator.md Outdated Show resolved Hide resolved
Comment on lines +19 to +21
Some Python packages we instrument or need in our instrumentation libraries,
might ship with some binary code. This is the case, for example, of `grpcio` and
`psutil` (used in `opentelemetry-instrumentation-system-metrics`).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Some Python packages we instrument or need in our instrumentation libraries,
might ship with some binary code. This is the case, for example, of `grpcio` and
`psutil` (used in `opentelemetry-instrumentation-system-metrics`).
Some Python packages we instrument or need in our instrumentation libraries
might ship with some binary code. This is the case, for example, with `grpcio` and
`psutil`, which are used in `opentelemetry-instrumentation-system-metrics`.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only psutil is used in the system metrics package, grpcio is used generally in the exporters or in its own instrumentation library.

Comment on lines +27 to +28
want to use it you might need to build your own image operator Docker image for
Python auto-instrumentation.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
want to use it you might need to build your own image operator Docker image for
Python auto-instrumentation.
want to use it, you might need to build your own image operator Docker image for
Python autoinstrumentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's spelled auto-instrumentation in other places

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly we have an issue for fixing that at once, I think it is fine to keep it that way

xrmx and others added 4 commits October 15, 2024 09:09
Co-authored-by: Tiffany Hrabusa <30397949+tiffany76@users.noreply.github.com>
Co-authored-by: Tiffany Hrabusa <30397949+tiffany76@users.noreply.github.com>
Co-authored-by: Tiffany Hrabusa <30397949+tiffany76@users.noreply.github.com>
@theletterf theletterf added this pull request to the merge queue Oct 15, 2024
Merged via the queue into open-telemetry:main with commit 71833a5 Oct 15, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Python specific quirks for kubernetes operator
7 participants