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.

Sign up for GitHub

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

Jump to bottom

set ELASTIC_APM_ACTIVATION_METHOD in code rather than values.yaml so it works for custom webhookConfig.agents.* names #91

Closed
trentm opened this issue Jan 11, 2024 · 1 comment · Fixed by #87
Closed

set ELASTIC_APM_ACTIVATION_METHOD in code rather than values.yaml so it works for custom webhookConfig.agents.* names #91

trentm opened this issue Jan 11, 2024 · 1 comment · Fixed by #87
Assignees
@trentm
Labels
apm-k8s-attacher Mutating Webhook to attach apm agents on k8s

Comments

@trentm
Copy link
Member

trentm commented Jan 11, 2024

Currently ELASTIC_APM_ACTIVATION_METHOD is set in webhookConfig.agents.{name} in the chart's values.yaml. E.g.:

webhookConfig:
  agents:
    java:
      image: docker.elastic.co/observability/apm-agent-java:latest
      artifact: "/usr/agent/elastic-apm-agent.jar"
      environment:
        JAVA_TOOL_OPTIONS: "-javaagent:/elastic/apm/agent/elastic-apm-agent.jar"
        ELASTIC_APM_ACTIVATION_METHOD: "K8S_ATTACH"

An issue with setting the envvar this was is that a custom webhookConfig.agents. name requires one to repeat that envvar. As noted at https://www.elastic.co/guide/en/apm/attacher/current/apm-webhook-add-pod-annotation.html

The only webhookConfig.agents values defined in values.yaml are java and nodejs. When using other values, you must explicitly specify image, artifact, and *OPTIONS values.

Instead, it would be more helpful to set that envvar always in the webhook code, then the config becomes simpler.

One requirement of doing this is that we need to use a common value for all APM agents. We decided to use the value currently being used by the Java agent: K8S_ATTACH. That means that new versions of the Node.js and .NET agents will be needed that support ELASTIC_APM_ACTIVATION_METHOD=K8S_ATTACH. I plan to change the Node.js APM agent to support both values so that the newer APM agent will work with current and newer versions of the Helm chart.
@trentm trentm self-assigned this Jan 11, 2024
@github-actions github-actions bot added the apm-k8s-attacher Mutating Webhook to attach apm agents on k8s label Jan 11, 2024
@trentm
Copy link
Member Author

trentm commented Jan 11, 2024

/cc @elastic/apm-agent-node-js @elastic/apm-agent-net

trentm added a commit that referenced this issue Jan 11, 2024
@trentm
set ELASTIC_APM_ACTIVATION_METHOD=K8S_ATTACH in code rather than valu…
a424344 
…es.yaml

This means that custom 'webhookConfig.agent.*' names do not need to repeat this
envvar. It *does* mean that newer Node.js and .NET APM agents that use the
newer value (used to be 'K8S') will be required for the 'activation_method' to
get set correctly.

Closes: #91
@trentm trentm mentioned this issue Jan 11, 2024
Merged
trentm added a commit to elastic/apm-agent-nodejs that referenced this issue Jan 11, 2024
@trentm
chore: support 'K8S_ATTACH' value for 'ELASTIC_APM_ACTIVATION_METHOD'
f58dc12 
This is a the new favoured value for the envvar to indicate the apm-k8s-attacher
is being used.
This should be backported.

Refs: elastic/apm-k8s-attacher#91
This was referenced Jan 11, 2024
Merged
Closed
trentm added a commit to elastic/apm-agent-nodejs that referenced this issue Jan 11, 2024
@trentm
chore: support 'K8S_ATTACH' value for 'ELASTIC_APM_ACTIVATION_METHOD' (…
7c0ba9d 
…#3819)

This is a the new favoured value for the envvar to indicate the apm-k8s-attacher
is being used.
This should be backported.

Refs: elastic/apm-k8s-attacher#91
trentm added a commit to elastic/apm-agent-nodejs that referenced this issue Jan 11, 2024
@trentm
chore: support 'K8S_ATTACH' value for 'ELASTIC_APM_ACTIVATION_METHOD' (…
6fcdf30 
…#3819)

(Backport of commit 7c0ba9d from main branch.)
This is a the new favoured value for the envvar to indicate the apm-k8s-attacher
is being used.
This should be backported.

Refs: elastic/apm-k8s-attacher#91
@trentm trentm mentioned this issue Jan 11, 2024
Merged
jackshirazi pushed a commit that referenced this issue Jan 12, 2024
@trentm
ELASTIC_APM_ACTIVATION_METHOD=K8S_ATTACH, prefer `ELASTIC_APM_SERVE…
7e30998 
…R_URL`; integration.yml workflow tweak (#87)

* test: drop redundant kind cluster creation in integration.yml workflow

* use ELASTIC_APM_SERVER_URL instead of the plural; the plural works with java, but not the Node.js or other APM agents

* set ELASTIC_APM_ACTIVATION_METHOD=K8S_ATTACH in code rather than values.yaml

This means that custom 'webhookConfig.agent.*' names do not need to repeat this
envvar. It *does* mean that newer Node.js and .NET APM agents that use the
newer value (used to be 'K8S') will be required for the 'activation_method' to
get set correctly.

Closes: #91

* fix tests

* improve the workflow step name
trentm added a commit to elastic/apm-agent-nodejs that referenced this issue Jan 12, 2024
@trentm
chore: support 'K8S_ATTACH' value for 'ELASTIC_APM_ACTIVATION_METHOD' (…
2d51eb8 
…#3819) (#3821)

(Backport of commit 7c0ba9d from main branch.)
This is a the new favoured value for the envvar to indicate the apm-k8s-attacher
is being used.
This should be backported.

Refs: elastic/apm-k8s-attacher#91
fpm-peter pushed a commit to fpm-git/apm-agent-nodejs that referenced this issue Aug 20, 2024
@trentm @fpm-peter
chore: support 'K8S_ATTACH' value for 'ELASTIC_APM_ACTIVATION_METHOD' (…
52b25f6 
…elastic#3819)

This is a the new favoured value for the envvar to indicate the apm-k8s-attacher
is being used.
This should be backported.

Refs: elastic/apm-k8s-attacher#91
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@trentm trentm

Labels
apm-k8s-attacher Mutating Webhook to attach apm agents on k8s
Projects
None yet
Milestone
No milestone
1 participant
@trentm