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

Apache auto-instrumentation #1082

Closed
wants to merge 13 commits into from

Conversation

chrlic
Copy link
Member

@chrlic chrlic commented Sep 8, 2022

PR proposes auto-instrumentation for applications running on Apache HTTPD server in K8S environment.
For instrumentation library, it uses https://github.com/open-telemetry/opentelemetry-cpp-contrib/tree/main/instrumentation/otel-webserver-module
For instrumentation itself, it adheres to the principles used for other languages already supported as much as possible.

@chrlic chrlic requested a review from a team as a code owner September 8, 2022 09:48
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 8, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@pavolloffay
Copy link
Member

I would propose to split the PR into:

  1. auto-instrumentation image and document the approach
  2. create injection functionality with e2e test
  3. setting the default version of auto-instrumentation in the operator

```

The Dockerfiles for auto-instrumentation can be found in [autoinstrumentation directory](./autoinstrumentation).
Follow the instructions in the Dockerfiles on how to build a custom container image.

>**Note:** For `Apache` auto-instrumentation, by default, instrumentation assumes httpd version 2.4. If you need to use version 2.2, or you need to adjust agent parameters, adjust instrumentation per following example:
Copy link
Member

Choose a reason for hiding this comment

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

This deserves a new section

Copy link
Member Author

Choose a reason for hiding this comment

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

Hello Pavol,

thank you for your review. Based on you comments, I'd do a new set of PR's

  1. auto-instrumentation image build and documentation
  2. changes related to Instrumentation CRD - definition, validation, etc.
  3. Injection itself + e2e tests
  4. default settings in the operator

Would that be OK?

Copy link
Member

Choose a reason for hiding this comment

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

yes, that would be perfect ❤️

- manager.yaml
Copy link
Member

Choose a reason for hiding this comment

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

this needs to be reverted

@jaronoff97
Copy link
Contributor

no longer needed! #1236 did this

@jaronoff97 jaronoff97 closed this Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants