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

CRD defs for Apache HTTPD Autoinstrumentation #1305

Merged

Conversation

chrlic
Copy link
Member

@chrlic chrlic commented Dec 6, 2022

This is a followup PR in Apache HTTPD Autoinstrumentation implementation project to previous PR #1236.

It contains CRD definition, defaults, validations, and test.

Next PR will contain the autoinstrumentation logic itself and e2e tests.

@chrlic chrlic requested a review from a team December 6, 2022 18:28
@chrlic
Copy link
Member Author

chrlic commented Dec 6, 2022

Updated with generated resources (make generate).

@chrlic
Copy link
Member Author

chrlic commented Dec 6, 2022

Generated bundle for Apache Httpd CRD (make bundle).

@chrlic
Copy link
Member Author

chrlic commented Dec 6, 2022

Version updated for make bundle

@chrlic
Copy link
Member Author

chrlic commented Dec 6, 2022

Chngnd repo user to open-telemetry for make bundle

// Attrs defines Apache HTTPD agent specific attributes. The precedence is:
// `agent default attributes` > `instrument spec attributes`
// +optional
Attrs []corev1.EnvVar `json:"attrs,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

what exactly is this configuring?

Copy link
Member Author

Choose a reason for hiding this comment

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

The OTel module for Apache httpd has number of configurable attributes (listed at https://github.com/open-telemetry/opentelemetry-cpp-contrib/tree/main/instrumentation/otel-webserver-module in Configuration section). There are reasonable defaults and some will be set by the instrumentation process, this gives a means to override those defaults.

Copy link
Member

Choose a reason for hiding this comment

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

Should we add this link to the godoc?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

config/manager/kustomization.yaml Outdated Show resolved Hide resolved
@frzifus
Copy link
Member

frzifus commented Dec 8, 2022

linting failed. its fixed in: 5e800c8

@chrlic chrlic requested a review from pavolloffay December 20, 2022 14:40

// Apache HTTPD server version. One of 2.4 or 2.2. Default is 2.4
// +optional
Version string `json:"version,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

for my curiosity how is this version used?

Copy link
Member Author

Choose a reason for hiding this comment

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

Apache HTTPD used these days is either version 2.2 or (most of the time) 2.4. The OTel agent is a set of binary loadable modules and those are compiled against specific Apache HTTPD version - different headers etc. Therefore you have to use the specific version of agent.

Copy link
Member Author

Choose a reason for hiding this comment

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

Based on the version setting, auto-instrumentation configures Apache HTTPD with the proper set of modules to be loaded. The docker image with the agent contains modules for both supported versions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Pavol,
could we move this forward, please?

Copy link
Member

Choose a reason for hiding this comment

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

merged, thanks ❤️

@chrlic chrlic requested a review from pavolloffay January 5, 2023 09:17
@chrlic
Copy link
Member Author

chrlic commented Jan 23, 2023

Hi Pavol,
could you please have a look at this?
BR,
Martin

@pavolloffay pavolloffay merged commit 05b871d into open-telemetry:main Feb 2, 2023
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
* CRD defs for Apache HTTPD Autoinstrumentation

* Generated resources for Apache Httpd CRD defs

* Generated bundle for Apache Httpd CRD

* Fixed version for "make bundle"

* Makefile and version update

* Chngnd repo user to open-telemetry for make bundle

* Reverted changes in kustomization.yaml

* Reverted changes in kustomization.yaml

* Reverted chnages in kustomization.yaml

* Apache HTTPD - Link to attributes doc

* make bundle sync

* ./.chloggen yaml added for open-telemetry#1305

* Version fix
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.

3 participants