WIP: Plug datadog apm #101
Conversation
| fieldRef: | ||
| fieldPath: status.hostIP | ||
| - name: DD_AGENT_SERVICE_PORT | ||
| value: '8126' |
There was a problem hiding this comment.
it should come from a variable?
| fieldRef: | ||
| fieldPath: status.hostIP | ||
| - name: DD_AGENT_SERVICE_PORT | ||
| value: '8126' |
There was a problem hiding this comment.
Should it be configurable, e.g. stored in a var?
| - name: "richie" | ||
| {% if ddtrace is defined and ddtrace %} | ||
| image: "docker-registry.default.svc:5000/{{ project_name }}/ddtrace-richie-{{ deployment_stamp }}:{{ richie_image_tag }}" | ||
| #image: "ddtrace-richie-{{ deployment_stamp }}:{{ richie_image_tag }}" |
There was a problem hiding this comment.
Why do you need to force docker registry address? Because of minishift?
Anyway, only the commented line should stay right?
There was a problem hiding this comment.
We discussed this with @rouja and propose to always point to an image stream.
Always having an image stream is an optimization we want because it stops downloading all the docker image layers on each deployment.
The syntax is more or less:
template:
spec:
containers:
- name: "richie"
image: ""
triggers:
- type: ConfigChange
- type: ImageChange
imageChangeParams:
automatic: true
containerNames:
- "richie"
from:
kind: ImageStreamTag
name: "{{ richie_image_name }}:{{ richie_image_tag }}"
Then in the image stream, we build the image we need for each project:
- adding monitoring (or not)
- for edxapp: adding a theme (or not)
- etc.
There was a problem hiding this comment.
That's a wonderful feature. I'll declare a new issue for this.
There was a problem hiding this comment.
@jmaupetit There is already a PR for this ;-) #105
There was a problem hiding this comment.
🤔 yes, but it only scopes edxapp right?
There was a problem hiding this comment.
True! But it's a functional POC whereas my comment above was a bit vague.
| # Switch back to the root user to install development dependencies | ||
| USER root:root | ||
|
|
||
| RUN pip install --no-cache-dir --prefix=/usr/local ddtrace |
There was a problem hiding this comment.
IMO the --prefix is not required: we do not need to split our dependencies in different places.
| - name: Set OpenShift datadog objects to manage | ||
| set_fact: | ||
| images: "{{ templates | map('regex_search', '.*/is.yml.j2$') | select('string') | list }}" | ||
| builds: "{{ templates | map('regex_search', '.*/bc.yml.j2$') | select('string') | list }}" |
There was a problem hiding this comment.
Those objects should be generic (imagestreams and buidconfigs), not only related to datadog APM. Hence you should move them in the next bloc.
| set_fact: | ||
| images: "{{ templates | map('regex_search', '.*/is.yml.j2$') | select('string') | list }}" | ||
| builds: "{{ templates | map('regex_search', '.*/bc.yml.j2$') | select('string') | list }}" | ||
| when: ddtrace |
There was a problem hiding this comment.
We always want to collect those objects not only when ddtrace is true. They are already conditionally created via their templates.
| - name: Display OpenShift's build for this app | ||
| debug: msg="{{ builds | to_nice_yaml}}" | ||
| when: builds is defined | ||
| when: ddtrace |
| tags: | ||
| - deploy | ||
| - image | ||
| - build |
There was a problem hiding this comment.
Same idea here: you should create ISs and BCs as generic objects. This should not be datadog-specific.
| # ddtrace is set to true for enable the APM in all apps where the templates | ||
| # include the possibility to have a ddtrace APM agent | ||
| # defaul is set to False | ||
| ddtrace: false |
There was a problem hiding this comment.
Should we name it is_apm_enabled instead of ddtrace which is less explicit?
|
|
||
| # ddtrace is set to true for enable the APM in all apps where the templates | ||
| # include the possibility to have a ddtrace APM agent | ||
| # defaul is set to False |
There was a problem hiding this comment.
I would write:
# Set xxx to "true" to enable APM in all apps where defined templates
# include this feature (default: "false")
Purpose
Add possibility to plug datadog APM to the apps. See #34.
Proposal
We must override the official docker images.
To do this we create a BuildConfig to add datadog APM (ddtrace) on service images.
The DeployementConfig use the OpenShift builded image.
We do this only if th ddtrace ansible vars is set to true.
edit: this PR is depends on #111