WIP: Add datadog APM support #115
Conversation
When Arnold clean a running stack labelled with a given deployment_stamp, we also need to remove created BuildConfigs.
APM layer must be enabled or disabled given particular env_types or customers. This can be achieved through the is_apm_enabled configuration. BuildConfigs for apps should use this variable to conditionally activate this feature during the build.
Enabling datadog APM can be summed as: * installing datadog agent * wrap the original command with the agent * add configs (env vars) + secrets
| @@ -29,7 +29,12 @@ spec: | |||
| source: | |||
| dockerfile: |- | |||
| FROM {{ richie_image_name }}:{{ richie_image_tag }} | |||
| # Add new statements here | |||
| USER 0 | |||
| {% if is_apm_enabled -%} | |||
There was a problem hiding this comment.
add a comment line because this Dockerfile extract will receive several separate augmentations?
| # include this feature (default: "false") | ||
| is_apm_enabled: false | ||
| # APM agent port defaults to Datadog agent service port | ||
| apm_agent_port: 8126 |
There was a problem hiding this comment.
Would be nice if we could omit the port when it is disabled.
There was a problem hiding this comment.
This is the default (main) value. When it's disabled, you don't need to declare this variable at all.
There was a problem hiding this comment.
See my other message, you can add a default value on the BC template
| @@ -29,7 +29,12 @@ spec: | |||
| source: | |||
| dockerfile: |- | |||
| FROM {{ richie_image_name }}:{{ richie_image_tag }} | |||
| # Add new statements here | |||
| USER 0 | |||
There was a problem hiding this comment.
Why creating a BC who do nothing if we have no apm ?
There was a problem hiding this comment.
To take advantage of the cluster's docker repository and speed up image pulling. We only create a IS/BC for our public images that will:
- will be updated frequently
- requires further customization given a
customerand anenv_type
| # Add new statements here | ||
| USER 0 | ||
| {% if is_apm_enabled -%} | ||
| RUN pip install ddtrace |
There was a problem hiding this comment.
This code is only for datadog so why the variable is named is_apm_enabled ?
If we use an other APM we need a new name.
I propose is_apm_datadog_enabled, if we add newrelic whe can use is_apm_newrelic_enabled.
Or we can specify the APM name on the variable, so s_apm_enabled can ave the value (false, datadog, newrelic, etc.)
| fieldRef: | ||
| fieldPath: status.hostIP | ||
| - name: DD_AGENT_SERVICE_PORT | ||
| value: "{{ apm_agent_port }}" |
There was a problem hiding this comment.
Can you add a default value ?
| # include this feature (default: "false") | ||
| is_apm_enabled: false | ||
| # APM agent port defaults to Datadog agent service port | ||
| apm_agent_port: 8126 |
There was a problem hiding this comment.
See my other message, you can add a default value on the BC template
|
@cchaudier Hence, Arnold does not have to be aware of the APM tool you choose. It's the app developer responsibility to implement such feature with datadog or any other tool/service. |
Purpose
This is a follow up of #101 considering recent changes concerning BuildConfig and ImageStream. We've decided to add APM to our apps by adding new layers to base images (via service BuildConfig).
Proposal
richieedxappCloses #101