isomerpages / isomercms-backend

A static website builder and host for the Singapore Government
5 stars 1 forks source link

fix(tags): update tagging for dd #1222

Closed seaerchin closed 7 months ago

seaerchin commented 7 months ago

Problem

previously, we updated env tags for production to prod but turns out it's being used in more than a few places. just change back to production + tag properly on docker labels also

Solution

  1. change container/run-time tags to production
  2. update docker labels
timotheeg commented 7 months ago

Thanks for that. Having the dd-tags in the task definition should help have the infra linked correctly in the APM service page too, so that's great.

But to with that said, I really think the -infra in service name is weird and confusing. Since we are only just starting to use DD heavily, let's have one session to finalize what we want things to be.

seaerchin commented 7 months ago

Thanks for that. Having the dd-tags in the task definition should help have the infra linked correctly in the PM service page too, so that's great.

But to with that said, I really think the -infra in service name is weird and confusing. Since we are only just starting to use DD heavily, let's have one session to finalize what we want things to be.

actually isomer-infra is required because we spin up and tag quite a number of stuff on pulumi. as the pulumi name is ${project}-${env} and we call the project isomer-infra, this leads to the tag being isomer-infra.

there's explicit dd tagging on the lambda/kinesis forwarder, but for the rest it might be difficult to re-tag if the tags are there already as pulumi will spin up new infra

timotheeg commented 7 months ago

actually isomer-infra is required because we spin up and tag quite a number of stuff on pulumi. as the pulumi name is ${project}-${env} and we call the project isomer-infra, this leads to the tag being isomer-infra.

I can see we called the project isomer-infra, but infra is just one aspect of a service we track, and so naming the project itself -infra is quite terrible and confusing 😢

I don't want to cause a lot of unnecessary rework though, but considering we have IaC, it should be quite easy to respin and change the service name, no? Let me book you tomorrow to discuss this.

Note that even ${project}-${env} is within our control btw, stack elements could be called ${project}-infra-${env} if we wanted to, without having to shove an -infra suffix in the service name itself 🤔.

timotheeg commented 7 months ago

For reference, most products do use the project name as itself (without a -infra suffix), which is typically the correct thing to do, examples below:

Obviously the concern on naming is not critical, and we could live with the service name isomer-infra! But since we're only at the beginning of doing monitoring "properly" on datadog, I'd say it's worth cleaning up. It will be more proper, and will avoid having recurring "why is the service name isomer-infra?" conversations, that pretty much any new engineer in the team will be asking 😅

harishv7 commented 7 months ago

For reference, most products do use the project name as itself (without a -infra suffix), which is typically the correct thing to do, examples below:

Obviously the concern on naming is not critical, and we could live with the service name isomer-infra! But since we're only at the beginning of doing monitoring "properly" on datadog, I'd say it's worth cleaning up. It will be more proper, and will avoid having recurring "why is the service name isomer-infra?" conversations, that pretty much any new engineer in the team will be asking 😅

@seaerchin @timotheeg I agree with Tim here, the suffix is confusing and unnecessary. Can we just change it to "isomer" perhaps?

seaerchin commented 7 months ago

@harishv7 @timotheeg - updated to isomer. as mentioned, the main concern was that pulumi would spin up new instances of stuff where the name was changed.

however, because ecs + logs have tagging set elsewhere, we can change it relatively pain-free. i'm still not 100% confident that all dd stuff will be tagged as isomer primarily due to pulumi components but we cna fix that as we go along