Skip to content

Commit ef066fd

Browse files
committed
refactor: optimize Spanner batching and standardize OpenTelemetry deployment
This commit optimizes Spanner write performance and standardizes OpenTelemetry (OTel) deployment across the backend API, background workers, and ingestion jobs. 1. Spanner Batching: * Refactored UpsertWPTRunFeatureMetrics in lib/gcpspanner/wpt_run_feature_metric.go to execute two batch queries at the transaction start instead of executing queries inside a loop, reducing database roundtrips. * Extracted mutation building to buildWPTRunFeatureMetricMutations to reduce cognitive complexity. 2. Centralized Terraform Telemetry Configuration: * Created root-level infra/telemetry.tf to define a single shared Secret Manager secret containing the OTel collector configuration. * Swapped the custom in-tree OTel collector image for the official Google-managed image (otelcol-google:0.151.0). This is to match this new documentation: https://docs.cloud.google.com/stackdriver/docs/instrumentation/opentelemetry-collector-cloud-run * Defined a centralized otel_collector_config_mount_path local variable in infra/telemetry.tf set to "/etc/otelcol-google" and propagated it to all submodules, replacing hardcoded paths. 3. OTel Sidecar Deployment (Go & Terraform): * Added opentelemetry.MaybeSetup helper to lib/opentelemetry/setup.go to encapsulate environment checks and OTel SDK initialization. * Refactored backend/cmd/server/main.go and all 11 background workers and daily ingestion scraper entrypoints to call MaybeSetup and defer shutdown. * Deployed the OTel sidecar container and mounted the shared config secret across all 4 worker pools and the reusable job module. * Granted roles/cloudtrace.agent, roles/monitoring.metricWriter, and roles/logging.logWriter to the backend and worker service accounts. 4. GCP Error Reporting & Structured Logging: * Updated the custom slog handler in lib/opentelemetry/slog.go to capture and append runtime/debug.Stack() to ERROR logs. * Structured logs to enable automatic GCP Error Reporting aggregation and trace-log linking using the trace field. 5. Go Startup Logging & Refactoring: * Added verbose BOOT: log statements before each client initialization phase (Datastore, Spanner, Valkey, OTel) in backend/cmd/server/main.go to provide startup phase visibility. * Refactored the inline OpenTelemetry setup block in main.go to use the new opentelemetry.MaybeSetup helper, keeping the startup sequence synchronous. 6. Repository Cleanup: * Deleted the unused custom in-tree OTel collector Dockerfile (otel/Dockerfile). * Removed the /otel Docker package-ecosystem update entry from .github/dependabot.yml. BUG=526562255
1 parent a5319d7 commit ef066fd

63 files changed

Lines changed: 2252 additions & 310 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/dependabot.yml

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,6 @@ updates:
7878
- '>=23'
7979
- '<24'
8080

81-
- package-ecosystem: 'docker'
82-
directory: '/otel'
83-
schedule:
84-
interval: 'weekly'
85-
8681
- package-ecosystem: 'docker'
8782
directory: '/.dev/auth'
8883
schedule:

backend/cmd/server/main.go

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ func main() {
5959
datastoreDB = &value
6060
}
6161
projectID := os.Getenv("PROJECT_ID")
62+
slog.InfoContext(context.TODO(), "BOOT: Creating Datastore client...")
6263
fs, err := gds.NewDatastoreClient(projectID, datastoreDB)
6364
if err != nil {
6465
slog.ErrorContext(context.TODO(), "failed to create datastore client", "error", err.Error())
@@ -69,6 +70,7 @@ func main() {
6970

7071
spannerDB := os.Getenv("SPANNER_DATABASE")
7172
spannerInstance := os.Getenv("SPANNER_INSTANCE")
73+
slog.InfoContext(ctx, "BOOT: Creating Spanner client...")
7274
spannerClient, err := gcpspanner.NewSpannerClient(projectID, spannerInstance, spannerDB)
7375
if err != nil {
7476
slog.ErrorContext(ctx, "failed to create spanner client", "error", err.Error())
@@ -106,6 +108,7 @@ func main() {
106108
slog.InfoContext(ctx, "cache settings", "duration", cacheTTL, "prefix", cacheKeyPrefix,
107109
"aggregatedFeaturesStatsTTL", aggregatedFeaturesStatsTTL)
108110

111+
slog.InfoContext(ctx, "BOOT: Creating Valkey cache...")
109112
cache, err := valkeycache.NewValkeyDataCache[string, []byte](
110113
cacheKeyPrefix,
111114
valkeyHost,
@@ -165,25 +168,22 @@ func main() {
165168
}),
166169
}
167170

168-
if os.Getenv("OTEL_SERVICE_NAME") != "" {
169-
slog.InfoContext(ctx, "opentelemetry settings detected.")
170-
otelProjectID := os.Getenv("OTEL_GCP_PROJECT_ID")
171-
if otelProjectID == "" {
172-
slog.ErrorContext(ctx, "missing project id for opentelemetry")
173-
os.Exit(1)
174-
}
175-
shutdown, err := opentelemetry.SetupOpenTelemetry(ctx, otelProjectID)
176-
if err != nil {
177-
slog.ErrorContext(ctx, "failed to setup opentelemetry", "error", err.Error())
178-
os.Exit(1)
179-
}
171+
slog.InfoContext(ctx, "BOOT: Running OTel setup...")
172+
shutdown, err := opentelemetry.MaybeSetup(ctx)
173+
if err != nil {
174+
slog.ErrorContext(ctx, "failed to setup opentelemetry", "error", err.Error())
175+
os.Exit(1)
176+
}
177+
if shutdown != nil {
180178
defer func() {
181-
err := shutdown(ctx)
182-
if err != nil {
183-
slog.ErrorContext(ctx, "unable to shutdown opentelemetry")
179+
if err := shutdown(ctx); err != nil {
180+
slog.ErrorContext(ctx, "unable to shutdown opentelemetry", "error", err)
184181
}
185182
}()
186-
// Prepend the opentelemtry middleware
183+
}
184+
185+
if os.Getenv("OTEL_SERVICE_NAME") != "" {
186+
// Prepend the opentelemetry middleware
187187
preRequestMiddlewares = slices.Insert(preRequestMiddlewares, 0, opentelemetry.NewOpenTelemetryChiMiddleware())
188188
}
189189

infra/backend/service.tf

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,11 @@ resource "docker_registry_image" "backend_remote_image" {
4646
data "google_project" "host_project" {
4747
}
4848

49-
module "otel_sidecar" {
50-
source = "../modules/otel"
51-
providers = {
52-
google.project = google.public_project
53-
}
54-
docker_repository_details = var.docker_repository_details
55-
service_account = google_service_account.backend.member
49+
resource "google_secret_manager_secret_iam_member" "backend_otel_config_secret_access" {
50+
provider = google.internal_project
51+
secret_id = var.otel_config_secret_id
52+
role = "roles/secretmanager.secretAccessor"
53+
member = "serviceAccount:${google_service_account.backend.email}"
5654
}
5755

5856
resource "google_cloud_run_v2_service" "service" {
@@ -150,7 +148,11 @@ resource "google_cloud_run_v2_service" "service" {
150148
}
151149
containers {
152150
name = "otel"
153-
image = module.otel_sidecar.otel_image
151+
image = var.otel_collector_image
152+
volume_mounts {
153+
name = "otel-config"
154+
mount_path = var.otel_collector_config_mount_path
155+
}
154156
liveness_probe {
155157
http_get {
156158
port = 4319
@@ -162,7 +164,7 @@ resource "google_cloud_run_v2_service" "service" {
162164
timeout_seconds = 10
163165
}
164166
startup_probe {
165-
initial_delay_seconds = 0
167+
initial_delay_seconds = 5
166168
timeout_seconds = 1
167169
period_seconds = 3
168170
failure_threshold = 10
@@ -178,6 +180,16 @@ resource "google_cloud_run_v2_service" "service" {
178180
}
179181
egress = "PRIVATE_RANGES_ONLY"
180182
}
183+
volumes {
184+
name = "otel-config"
185+
secret {
186+
secret = var.otel_config_secret_id
187+
items {
188+
version = "latest"
189+
path = "config.yaml"
190+
}
191+
}
192+
}
181193
service_account = google_service_account.backend.email
182194
}
183195

@@ -188,6 +200,7 @@ resource "google_cloud_run_v2_service" "service" {
188200

189201
depends_on = [
190202
google_project_iam_member.gcp_datastore_user,
203+
google_secret_manager_secret_iam_member.backend_otel_config_secret_access,
191204
]
192205

193206
deletion_protection = var.deletion_protection
@@ -327,3 +340,29 @@ resource "google_compute_managed_ssl_certificate" "lb_default" {
327340
domains = var.domains
328341
}
329342
}
343+
344+
# --- Telemetry IAM Permissions for Backend Service Account ---
345+
346+
# Grant Cloud Trace Agent role to allow exporting traces directly or via OTel sidecar.
347+
resource "google_project_iam_member" "backend_trace_agent" {
348+
provider = google.public_project
349+
project = var.projects.public
350+
role = "roles/cloudtrace.agent"
351+
member = "serviceAccount:${google_service_account.backend.email}"
352+
}
353+
354+
# Grant Monitoring Metric Writer role to allow exporting metrics.
355+
resource "google_project_iam_member" "backend_metric_writer" {
356+
provider = google.public_project
357+
project = var.projects.public
358+
role = "roles/monitoring.metricWriter"
359+
member = "serviceAccount:${google_service_account.backend.email}"
360+
}
361+
362+
# Grant Logging Log Writer role to allow exporting structured logs.
363+
resource "google_project_iam_member" "backend_log_writer" {
364+
provider = google.public_project
365+
project = var.projects.public
366+
role = "roles/logging.logWriter"
367+
member = "serviceAccount:${google_service_account.backend.email}"
368+
}

infra/backend/variables.tf

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,18 @@ variable "backend_api_url" {
110110

111111
variable "pubsub_project_id" { type = string }
112112
variable "ingestion_topic_id" { type = string }
113+
114+
variable "otel_config_secret_id" {
115+
type = string
116+
description = "The Secret Manager secret ID containing the OTel collector configuration"
117+
}
118+
119+
variable "otel_collector_image" {
120+
type = string
121+
description = "The container image to use for the OTel collector sidecar"
122+
}
123+
124+
variable "otel_collector_config_mount_path" {
125+
type = string
126+
description = "The volume mount path for the OTel collector configuration"
127+
}

infra/ingestion/variables.tf

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,3 +94,23 @@ variable "notification_channel_ids" {
9494
description = "A list of notification channel ids to send alerts to."
9595
type = list(string)
9696
}
97+
98+
variable "otel_config_secret_id" {
99+
type = string
100+
description = "The Secret Manager secret ID containing the OTel collector configuration"
101+
}
102+
103+
variable "otel_project_id" {
104+
type = string
105+
description = "The GCP project ID where telemetry traces/metrics will be exported"
106+
}
107+
108+
variable "otel_collector_image" {
109+
type = string
110+
description = "The container image to use for the OTel collector sidecar"
111+
}
112+
113+
variable "otel_collector_config_mount_path" {
114+
type = string
115+
description = "The volume mount path for the OTel collector configuration"
116+
}

infra/ingestion/workflows.tf

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ module "web_features_workflow" {
5959
value = "262980h" # 30 years in hours (365.25*30*24)
6060
}
6161
]
62+
otel_config_secret_id = var.otel_config_secret_id
63+
otel_project_id = var.otel_project_id
64+
otel_collector_image = var.otel_collector_image
65+
otel_collector_config_mount_path = var.otel_collector_config_mount_path
6266
}
6367

6468
module "wpt_workflow" {
@@ -105,9 +109,13 @@ module "wpt_workflow" {
105109
},
106110
{
107111
name = "DATA_WINDOW_DURATION"
108-
value = "17520h" # 2 years
112+
value = "720h" # 30 days
109113
}
110114
]
115+
otel_config_secret_id = var.otel_config_secret_id
116+
otel_project_id = var.otel_project_id
117+
otel_collector_image = var.otel_collector_image
118+
otel_collector_config_mount_path = var.otel_collector_config_mount_path
111119
}
112120

113121
module "bcd_workflow" {
@@ -144,6 +152,10 @@ module "bcd_workflow" {
144152
value = var.spanner_datails.instance
145153
}
146154
]
155+
otel_config_secret_id = var.otel_config_secret_id
156+
otel_project_id = var.otel_project_id
157+
otel_collector_image = var.otel_collector_image
158+
otel_collector_config_mount_path = var.otel_collector_config_mount_path
147159
}
148160

149161
module "chromium_enum_workflow" {
@@ -180,6 +192,10 @@ module "chromium_enum_workflow" {
180192
value = var.spanner_datails.instance
181193
}
182194
]
195+
otel_config_secret_id = var.otel_config_secret_id
196+
otel_project_id = var.otel_project_id
197+
otel_collector_image = var.otel_collector_image
198+
otel_collector_config_mount_path = var.otel_collector_config_mount_path
183199
}
184200

185201
module "uma_export_workflow" {
@@ -216,6 +232,10 @@ module "uma_export_workflow" {
216232
value = var.spanner_datails.instance
217233
}
218234
]
235+
otel_config_secret_id = var.otel_config_secret_id
236+
otel_project_id = var.otel_project_id
237+
otel_collector_image = var.otel_collector_image
238+
otel_collector_config_mount_path = var.otel_collector_config_mount_path
219239
}
220240

221241
module "developer_signals_workflow" {
@@ -252,6 +272,10 @@ module "developer_signals_workflow" {
252272
value = var.spanner_datails.instance
253273
}
254274
]
275+
otel_config_secret_id = var.otel_config_secret_id
276+
otel_project_id = var.otel_project_id
277+
otel_collector_image = var.otel_collector_image
278+
otel_collector_config_mount_path = var.otel_collector_config_mount_path
255279
}
256280

257281
module "web_features_mapping_workflow" {
@@ -288,4 +312,8 @@ module "web_features_mapping_workflow" {
288312
value = var.spanner_datails.instance
289313
}
290314
]
315+
otel_config_secret_id = var.otel_config_secret_id
316+
otel_project_id = var.otel_project_id
317+
otel_collector_image = var.otel_collector_image
318+
otel_collector_config_mount_path = var.otel_collector_config_mount_path
291319
}

infra/main.tf

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ module "ingestion" {
9696
developer_signals_region_schedules = var.developer_signals_region_schedules
9797
web_features_mapping_region_schedules = var.web_features_mapping_region_schedules
9898
notification_channel_ids = var.notification_channel_ids
99+
otel_config_secret_id = google_secret_manager_secret.otel_config.id
100+
otel_project_id = var.projects.public
101+
otel_collector_image = local.otel_collector_image
102+
otel_collector_config_mount_path = local.otel_collector_config_mount_path
99103
}
100104

101105
module "backend" {
@@ -124,8 +128,11 @@ module "backend" {
124128
firebase_settings = {
125129
tenant_id = module.auth.tenant_id
126130
}
127-
pubsub_project_id = var.projects.internal
128-
ingestion_topic_id = module.pubsub.ingestion_topic_id
131+
pubsub_project_id = var.projects.internal
132+
ingestion_topic_id = module.pubsub.ingestion_topic_id
133+
otel_config_secret_id = google_secret_manager_secret.otel_config.id
134+
otel_collector_image = local.otel_collector_image
135+
otel_collector_config_mount_path = local.otel_collector_config_mount_path
129136
}
130137

131138
module "frontend" {
@@ -205,6 +212,10 @@ module "workers" {
205212

206213
chime_details = var.chime_details
207214

208-
email_service_account_email = var.email_service_account_email
209-
deletion_protection = var.deletion_protection
215+
email_service_account_email = var.email_service_account_email
216+
deletion_protection = var.deletion_protection
217+
otel_config_secret_id = google_secret_manager_secret.otel_config.id
218+
otel_project_id = var.projects.public
219+
otel_collector_image = local.otel_collector_image
220+
otel_collector_config_mount_path = local.otel_collector_config_mount_path
210221
}

infra/modules/job/main.tf

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,33 @@ resource "google_cloud_run_v2_job" "job" {
3636
value = env.value.value
3737
}
3838
}
39+
env {
40+
name = "OTEL_SERVICE_NAME"
41+
value = var.short_name
42+
}
43+
env {
44+
name = "OTEL_GCP_PROJECT_ID"
45+
value = var.otel_project_id
46+
}
47+
}
48+
containers {
49+
name = "otel"
50+
image = var.otel_collector_image
51+
volume_mounts {
52+
name = "otel-config"
53+
mount_path = var.otel_collector_config_mount_path
54+
}
55+
# No probes for Cloud Run Jobs as they run to completion
56+
}
57+
volumes {
58+
name = "otel-config"
59+
secret {
60+
secret = var.otel_config_secret_id
61+
items {
62+
version = "latest"
63+
path = "config.yaml"
64+
}
65+
}
3966
}
4067
service_account = google_service_account.job_service_account.email
4168
}
@@ -68,3 +95,28 @@ resource "google_project_iam_member" "datastore_user" {
6895
member = google_service_account.job_service_account.member
6996
}
7097

98+
# --- Telemetry IAM Permissions for Ingestion Job Service Accounts ---
99+
100+
# Grant Cloud Trace Agent role to allow exporting traces.
101+
resource "google_project_iam_member" "job_trace_agent" {
102+
provider = google.internal_project
103+
project = var.spanner_project_id
104+
role = "roles/cloudtrace.agent"
105+
member = google_service_account.job_service_account.member
106+
}
107+
108+
# Grant Monitoring Metric Writer role to allow exporting metrics.
109+
resource "google_project_iam_member" "job_metric_writer" {
110+
provider = google.internal_project
111+
project = var.spanner_project_id
112+
role = "roles/monitoring.metricWriter"
113+
member = google_service_account.job_service_account.member
114+
}
115+
116+
# Grant Logging Log Writer role to allow exporting structured logs.
117+
resource "google_project_iam_member" "job_log_writer" {
118+
provider = google.internal_project
119+
project = var.spanner_project_id
120+
role = "roles/logging.logWriter"
121+
member = google_service_account.job_service_account.member
122+
}

0 commit comments

Comments
 (0)