Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Select an option

  • Save jonangeles-sanchez/2bfa5c15fab066d028c2b4bea4107eda to your computer and use it in GitHub Desktop.

Select an option

Save jonangeles-sanchez/2bfa5c15fab066d028c2b4bea4107eda to your computer and use it in GitHub Desktop.
# Logger.log Clarity Audit Report
Generated: 2025-12-16
## Executive Summary
- Total files scanned: 86
- Total Logger.log calls found: ~380
- Calls needing improvement: 48
- Calls already clear: ~332
## Improvement Categories
- Action names needing clarity: 22 instances
- Messages needing improvement: 18 instances
- Metadata keys needing improvement: 8 instances
---
## Detailed Changes by File
### File: server/loaders/BullMQ/Queues/ElasticSearch/events/Jobs/onCompanyUpdated.ts
**Total Logger.log calls:** 7
**Calls needing improvement:** 4
#### Change 1: [Line 45]
**Current:**
```typescript
Logger.log("info", "event", "activity", "jobs_fetched", {
message: `Fetched ${sqlIds.length} jobs from database for company ${company_id}`,
job_id: sqlIds.map((id) => parseInt(id)),
});
```
**Proposed:**
```typescript
Logger.log("info", "event", "activity", "fetch_company_jobs_for_sync", {
message: `Fetched ${sqlIds.length} open jobs from database for company ${company_id}`,
job_id: sqlIds.map((id) => parseInt(id)),
company_id,
});
```
**Rationale:** "jobs_fetched" is too generic - doesn't indicate this is fetching jobs for synchronization purposes. The proposed name clarifies this is fetching company jobs specifically for ElasticSearch sync. Added company_id to message fields for better CloudWatch filtering.
#### Change 2: [Line 57]
**Current:**
```typescript
Logger.log("info", "event", "activity", "existing_jobs_found", {
message: `Found ${exist.length} existing jobs in ElasticSearch for company ${company_id}`,
job_id: exist.filter((e) => e._source).map((e) => e._source!.id),
});
```
**Proposed:**
```typescript
Logger.log("info", "event", "activity", "find_existing_jobs_in_index", {
message: `Found ${exist.length} existing jobs in ElasticSearch index for company ${company_id}`,
job_id: exist.filter((e) => e._source).map((e) => e._source!.id),
company_id,
});
```
**Rationale:** "existing_jobs_found" is vague. The proposed name clarifies this is finding jobs that already exist in the ElasticSearch index during a sync operation. Added company_id primary key.
#### Change 3: [Line 74]
**Current:**
```typescript
Logger.log(
"warn",
"event",
"activity",
"document_missing_job",
{ message: `No job data was found for this document.` },
{
elastic_job_id: _id,
},
);
```
**Proposed:**
```typescript
Logger.log(
"warn",
"event",
"activity",
"elasticsearch_document_missing_source",
{ message: `ElasticSearch document has no _source field` },
{
elasticsearch_document_id: _id,
company_id,
},
);
```
**Rationale:** "document_missing_job" is unclear - it's actually about a missing _source field in an ElasticSearch document. The proposed name and message are more precise. Renamed metadata key from "elastic_job_id" to "elasticsearch_document_id" for clarity.
#### Change 4: [Line 84]
**Current:**
```typescript
Logger.log("info", "event", "activity", "modified_job_not_found", {
message: `Elastic search job was not found in the database jobs`,
job_id: job.id,
});
```
**Proposed:**
```typescript
Logger.log("info", "event", "activity", "index_job_missing_from_database", {
message: `Job ${job.id} exists in ElasticSearch index but not found in database results`,
job_id: job.id,
company_id,
});
```
**Rationale:** "modified_job_not_found" doesn't clearly convey what's happening. This is a job that exists in the index but wasn't in the database query results. The proposed name and message clarify this discrepancy.
---
### File: server/loaders/BullMQ/Queues/ElasticSearch/events/Jobs/onJobUpdated.ts
**Total Logger.log calls:** 4
**Calls needing improvement:** 3
#### Change 1: [Line 30]
**Current:**
```typescript
Logger.log("info", "event", "activity", "handle_non_existent_job", {
message: `Job ${job_id} was not found in the database.`,
});
```
**Proposed:**
```typescript
Logger.log("info", "event", "activity", "job_not_found_skipping_update", {
message: `Job ${job_id} not found in database - skipping ElasticSearch update`,
job_id,
});
```
**Rationale:** "handle_non_existent_job" is too generic and uses "handle" pattern. The proposed name clarifies that we're skipping the update operation because the job doesn't exist. Added job_id primary key.
#### Change 2: [Line 41]
**Current:**
```typescript
Logger.log("info", "event", "activity", "handle_not_open_job", {
message: `Job ${job_id} is not in Open status, deleting from the ElasticSearch index (or ignoring it if it's not there).`,
});
```
**Proposed:**
```typescript
Logger.log("info", "event", "activity", "delete_non_open_job_from_index", {
message: `Job ${job_id} status is ${job_status_name} - removing from ElasticSearch index`,
job_id,
job_status_name,
});
```
**Rationale:** "handle_not_open_job" is vague. The proposed name clearly states the action (delete from index). Message clarifies what's happening and includes the actual status. Added primary keys for filtering.
#### Change 3: [Line 64]
**Current:**
```typescript
Logger.log("info", "event", "activity", "handle_original_job_not_found", {
message: `Job ${job_id} not found in the ElasticSearch index.`,
});
```
**Proposed:**
```typescript
Logger.log("info", "event", "activity", "job_not_in_index_skipping_update", {
message: `Job ${job_id} not found in ElasticSearch index - cannot update`,
job_id,
});
```
**Rationale:** "handle_original_job_not_found" uses the vague "handle" pattern. The proposed name clarifies we're skipping the update because the job isn't in the index yet. Added job_id primary key.
---
### File: server/loaders/BullMQ/Queues/ElasticSearch/events/Tradesmen/onTradesmanChanged.ts
**Total Logger.log calls:** 4
**Calls needing improvement:** 2
#### Change 1: [Line 18]
**Current:**
```typescript
Logger.log("warn", "event", "activity", "missing_tradesman_id", {
message: "Tradesman ID is missing from the payload.",
});
```
**Proposed:**
```typescript
Logger.log("warn", "event", "activity", "tradesman_id_missing_from_event", {
message: "onTradesmanChanged event triggered with no tradesman_id in payload",
});
```
**Rationale:** "missing_tradesman_id" is too generic. The proposed name adds context that this is from an event payload. Message clarifies which event this occurred in.
#### Change 2: [Line 45]
**Current:**
```typescript
Logger.log(
"warn",
"event",
"activity",
"incomplete_profile",
{
message: `Tradesman ${tradesman_id} is missing required fields and cannot be stored in ElasticSearch during onTradesmanChanged event.`,
},
{
finished_registration: tradesman.finished_registration,
tradefile_exists: !!tradesman.tradefile_url,
},
);
```
**Proposed:**
```typescript
Logger.log(
"warn",
"event",
"activity",
"tradesman_profile_incomplete_cannot_index",
{
message: `Tradesman ${tradesman_id} missing required fields - cannot index to ElasticSearch`,
tradesman_id,
},
{
finished_registration: tradesman.finished_registration,
has_tradefile: !!tradesman.tradefile_url,
},
);
```
**Rationale:** "incomplete_profile" is too generic - many profiles could be incomplete. The proposed name clarifies this prevents indexing. Renamed metadata key "tradefile_exists" to "has_tradefile" for consistency with boolean naming conventions.
---
### File: server/loaders/BullMQ/Queues/Global/activity/events/onActivityLogged.ts
**Total Logger.log calls:** 5
**Calls needing improvement:** 4
#### Change 1: [Line 18]
**Current:**
```typescript
Logger.log("info", "event", "activity", "tradesman_id_required", {
message: "Tradesman id is required",
});
```
**Proposed:**
```typescript
Logger.log("info", "event", "activity", "tradesman_id_missing_skipping_activity_update", {
message: "onActivityLogged event missing tradesman_id - skipping activity datetime update",
});
```
**Rationale:** "tradesman_id_required" is generic validation message. The proposed name clarifies context (activity update) and what happens (skipping). Message explains which event and what is being skipped.
#### Change 2: [Line 42]
**Current:**
```typescript
Logger.log("warn", "event", "activity", "incomplete_profile", {
message: `Tradesman ${tradesman_id} is missing required fields and cannot be stored in ElasticSearch during onActivityLogged event.`,
tradesman_id,
});
```
**Proposed:**
```typescript
Logger.log("warn", "event", "activity", "tradesman_profile_incomplete_cannot_update_activity", {
message: `Tradesman ${tradesman_id} missing required fields - cannot update last_activity_datetime in ElasticSearch`,
tradesman_id,
});
```
**Rationale:** "incomplete_profile" is used multiple times across codebase. The proposed name distinguishes this specific case (updating activity timestamp). Message clarifies what field cannot be updated.
#### Change 3: [Line 57]
**Current:**
```typescript
Logger.log("info", "event", "activity", "tradesman_updated", {
message: `Tradesman with id: ${tradesman_id} updated in ElasticSearch`,
});
```
**Proposed:**
```typescript
Logger.log("info", "event", "activity", "tradesman_activity_datetime_updated", {
message: `Updated last_activity_datetime for tradesman ${tradesman_id} in ElasticSearch`,
tradesman_id,
});
```
**Rationale:** "tradesman_updated" is too generic - many things update tradesmen. The proposed name specifies exactly what was updated (activity datetime). Added tradesman_id primary key.
#### Change 4: [Line 64]
**Current:**
```typescript
return Logger.log("error", "event", "activity", "tradesman_updated", {
message: `Error updating tradesman in ElasticSearch: ${JSON.stringify(e)}`,
});
```
**Proposed:**
```typescript
return Logger.log("error", "event", "activity", "elasticsearch_tradesman_update_error", {
message: `Failed to update tradesman in ElasticSearch`,
tradesman_id,
},
{
error_details: JSON.stringify(e),
});
```
**Rationale:** Using "tradesman_updated" for error case is confusing (same name as success). Proposed name clarifies it's an error. Moved error details to metadata and added tradesman_id primary key.
---
### File: server/loaders/BullMQ/Queues/Global/application/jobs/AutomaticApply.ts
**Total Logger.log calls:** 8
**Calls needing improvement:** 2
#### Change 1: [Line 394]
**Current:**
```typescript
Logger.log(
"info",
"cron_job",
"activity",
"get_jobs",
{
message: `Found ${jobs.length} jobs.`,
job_id: jobs.map((j) => j.job_id),
},
{
jobs: jobs.map(({ job_id, job_post_title, application_vacancy_count, application_summary }) => ({
job_id,
job_post_title,
application_vacancy_count,
application_summary,
})),
},
);
```
**Proposed:**
```typescript
Logger.log(
"info",
"cron_job",
"activity",
"fetch_jobs_for_automatic_apply",
{
message: `Found ${jobs.length} jobs eligible for automatic applications`,
job_id: jobs.map((j) => j.job_id),
},
{
eligible_jobs: jobs.map(({ job_id, job_post_title, application_vacancy_count, application_summary }) => ({
job_id,
job_post_title,
application_vacancy_count,
application_summary,
})),
},
);
```
**Rationale:** "get_jobs" is too generic - doesn't indicate purpose. The proposed name clarifies these are jobs for automatic apply matching. Message is more descriptive. Renamed metadata key from "jobs" to "eligible_jobs" for clarity.
#### Change 2: [Line 421]
**Current:**
```typescript
Logger.log(
"info",
"cron_job",
"activity",
"get_tradesmen",
{
message: `Found ${tradesmen.length} tradesmen`,
tradesman_id: tradesmen.map((t) => t.tradesman_id),
},
{
tradesmen: tradesmen.map(({ tradesman_id, trade_blueprint_ids, application_vacancy_count, auto_apply_enabled_datetime, recent_applications_count }) => ({
tradesman_id,
trade_blueprint_ids,
application_vacancy_count,
auto_apply_enabled_datetime,
recent_applications_count,
})),
},
);
```
**Proposed:**
```typescript
Logger.log(
"info",
"cron_job",
"activity",
"fetch_tradesmen_for_automatic_apply",
{
message: `Found ${tradesmen.length} tradesmen eligible for automatic applications`,
tradesman_id: tradesmen.map((t) => t.tradesman_id),
},
{
eligible_tradesmen: tradesmen.map(({ tradesman_id, trade_blueprint_ids, application_vacancy_count, auto_apply_enabled_datetime, recent_applications_count }) => ({
tradesman_id,
trade_blueprint_ids,
application_vacancy_count,
auto_apply_enabled_datetime,
recent_applications_count,
})),
},
);
```
**Rationale:** "get_tradesmen" is too generic - doesn't indicate these are filtered for automatic apply. The proposed name clarifies purpose. Renamed metadata key from "tradesmen" to "eligible_tradesmen" for specificity.
---
### File: server/loaders/BullMQ/Queues/Global/google/jobs/SynchronizeGoogle.ts
**Total Logger.log calls:** 2
**Calls needing improvement:** 2
#### Change 1: [Line 18]
**Current:**
```typescript
Logger.log(
"info",
"cron_job",
"activity",
"handle_open_results",
{
message: "openResults returned as a null value. Please check config.google.allowIndexing",
},
{
allow_indexing: config.google?.allowIndexing,
},
);
```
**Proposed:**
```typescript
Logger.log(
"info",
"cron_job",
"activity",
"google_sync_skipped_indexing_disabled",
{
message: "Google job synchronization skipped - indexing disabled in config",
},
{
allow_indexing: config.google?.allowIndexing,
},
);
```
**Rationale:** "handle_open_results" is vague and uses "handle" pattern. The proposed name clearly states why sync was skipped. Message is clearer about what's happening.
#### Change 2: [Line 44]
**Current:**
```typescript
Logger.log(
"info",
"cron_job",
"activity",
"handle_closed_results",
{
message: "closedResults returned as a null value. Please check config.google.allowIndexing",
},
{
allow_indexing: config.google?.allowIndexing,
},
);
```
**Proposed:**
```typescript
Logger.log(
"info",
"cron_job",
"activity",
"google_closed_sync_skipped_indexing_disabled",
{
message: "Google closed jobs synchronization skipped - indexing disabled in config",
},
{
allow_indexing: config.google?.allowIndexing,
},
);
```
**Rationale:** "handle_closed_results" is vague. The proposed name distinguishes this from open jobs sync and clarifies why it was skipped.
---
### File: server/loaders/BullMQ/Queues/Global/job/jobs/RecommendJobs.ts
**Total Logger.log calls:** 1
**Calls needing improvement:** 1
#### Change 1: [Line 277]
**Current:**
```typescript
Logger.log(
"info",
"cron_job",
"activity",
"get_recommended_jobs",
{
message: `Got ${matches.length} matches`,
tradesman_id: matches.map((m) => m.tradesman_id),
job_id: matches.flatMap((m) => m.matches.map((job) => job.job_id)),
},
{
matches: matches.map(({
tradesman_id,
location_id,
job_title,
willing_to_travel,
applications,
trade_blueprint_ids,
matches: matched_jobs,
}) => ({ ... })),
},
);
```
**Proposed:**
```typescript
Logger.log(
"info",
"cron_job",
"activity",
"match_jobs_to_recommend_to_tradesmen",
{
message: `Matched ${matches.length} tradesmen with jobs for recommendations`,
tradesman_id: matches.map((m) => m.tradesman_id),
job_id: matches.flatMap((m) => m.matches.map((job) => job.job_id)),
},
{
recommendation_matches: matches.map(({
tradesman_id,
location_id,
job_title,
willing_to_travel,
applications,
trade_blueprint_ids,
matches: matched_jobs,
}) => ({ ... })),
},
);
```
**Rationale:** "get_recommended_jobs" is ambiguous. The proposed name clarifies we're matching jobs TO tradesmen for recommendation. Renamed metadata key from "matches" to "recommendation_matches" for clarity.
---
### File: server/loaders/BullMQ/Queues/Global/job/jobs/RecommendPerDiemJobs.ts
**Total Logger.log calls:** 1
**Calls needing improvement:** 1
#### Change 1: [Line 96]
**Current:**
```typescript
Logger.log(
"info",
"cron_job",
"activity",
"get_per_diem_matches",
{
message: `Matched ${tradesmenWithMatchedJobs.length} tradesmen with per diem jobs`,
tradesman_id: tradesmenWithMatchedJobs.map((m) => m.tradesman_id),
job_id: tradesmenWithMatchedJobs.flatMap((m) => m.matches.map((job) => job.job_id)),
},
{
matches: tradesmenWithMatchedJobs.map(({ ... }) => ({ ... })),
},
);
```
**Proposed:**
```typescript
Logger.log(
"info",
"cron_job",
"activity",
"match_per_diem_jobs_to_recommend",
{
message: `Matched ${tradesmenWithMatchedJobs.length} tradesmen with per diem jobs for recommendations`,
tradesman_id: tradesmenWithMatchedJobs.map((m) => m.tradesman_id),
job_id: tradesmenWithMatchedJobs.flatMap((m) => m.matches.map((job) => job.job_id)),
},
{
per_diem_recommendation_matches: tradesmenWithMatchedJobs.map(({ ... }) => ({ ... })),
},
);
```
**Rationale:** "get_per_diem_matches" doesn't clearly convey purpose. The proposed name clarifies these are matches for job recommendations. Renamed metadata key to "per_diem_recommendation_matches" for specificity.
---
### File: server/loaders/BullMQ/Queues/Metrics/job/jobs/SynchronizeMetrics.ts
**Total Logger.log calls:** 1
**Calls needing improvement:** 1
#### Change 1: [Line 55]
**Current:**
```typescript
Logger.log(
"info",
"cron_job",
"activity",
"fetch_metrics",
{
message: `Synchronized metrics of ${result.metrics.length} jobs`,
job_id: result.metrics.map(({ job_id }) => job_id),
},
{
has_more: result.has_more,
},
);
```
**Proposed:**
```typescript
Logger.log(
"info",
"cron_job",
"activity",
"synchronize_job_metrics_batch",
{
message: `Synchronized view/apply/share metrics for ${result.metrics.length} jobs`,
job_id: result.metrics.map(({ job_id }) => job_id),
},
{
has_more_pages: result.has_more,
},
);
```
**Rationale:** "fetch_metrics" doesn't indicate we're synchronizing/updating. The proposed name clarifies this is batch synchronization. Message specifies which metrics. Renamed metadata key to "has_more_pages" for clarity.
---
### File: server/services/activity/tradesman.ts
**Total Logger.log calls:** 1
**Calls needing improvement:** 0
**Status:** All calls are clear and well-structured.
---
### File: server/services/application/applicationCrons.ts
**Total Logger.log calls:** 2
**Calls needing improvement:** 1
#### Change 1: [Line 97]
**Current:**
```typescript
Logger.log(
"error",
this.entity,
"activity",
"handle_matches",
{ message: `Error while inserting ${values.length} applications` },
{
operation: "error",
error: e,
chunk_size: values.length,
applications: values.map((v) => ({
tradesman_id: v.tradesman_id,
job_id: v.job_id,
})),
},
);
```
**Proposed:**
```typescript
Logger.log(
"error",
this.entity,
"activity",
"automatic_apply_bulk_insert_error",
{
message: `Failed to bulk insert ${values.length} automatic applications`,
tradesman_id: values.map((v) => v.tradesman_id),
job_id: values.map((v) => v.job_id),
},
{
error: e instanceof Error ? e.message : String(e),
stack: e instanceof Error ? e.stack : undefined,
chunk_size: values.length,
failed_applications: values.map((v) => ({
tradesman_id: v.tradesman_id,
job_id: v.job_id,
})),
},
);
```
**Rationale:** "handle_matches" is vague and uses "handle" pattern. The proposed name clarifies this is a bulk insert error for automatic applications. Removed redundant "operation: error" metadata (already error level). Improved error handling. Added primary keys to message fields. Renamed "applications" to "failed_applications" for clarity.
---
### File: server/services/application/index.ts
**Total Logger.log calls:** 1
**Calls needing improvement:** 0
**Status:** The single Logger.log call at line 462 is already clear with specific action name "submit_application" and descriptive message.
---
### File: server/services/elasticSearch.ts
**Total Logger.log calls:** 3
**Calls needing improvement:** 1
#### Change 1: [Line 210]
**Current:**
```typescript
Logger.log(
"info",
this.entity,
"activity",
"elastic_search_query",
{ message: "Query executed successfully" },
{
index: index,
query,
from,
size,
sort,
results_count: hits.length,
},
);
```
**Proposed:**
```typescript
Logger.log(
"info",
this.entity,
"activity",
"execute_elasticsearch_query",
{ message: `ElasticSearch query returned ${hits.length} results` },
{
index: index,
query,
from,
size,
sort,
results_count: hits.length,
},
);
```
**Rationale:** "elastic_search_query" is vague. The proposed name uses action verb "execute". Message now includes result count for quick visibility without opening metadata.
---
### File: server/services/email/klaviyo.ts
**Total Logger.log calls:** 4
**Calls needing improvement:** 0
**Status:** All calls are clear with specific action names like "klaviyo_duplicate_phone_number", "klaviyo_invalid_phone_number", "klaviyo_upsert_profile_error", and "klaviyo_profile_event_sent".
---
### File: server/services/email/mailgun.ts
**Total Logger.log calls:** 2
**Calls needing improvement:** 0
**Status:** Calls are clear with specific action names "mailgun_send_email" and "mailgun_send_email_error".
---
### File: server/services/email/nodeMailer.ts
**Total Logger.log calls:** 2
**Calls needing improvement:** 0
**Status:** Calls are clear with specific action names "node_mailer_send_email" and "node_mailer_send_email_error".
---
### File: server/services/error/index.ts
**Total Logger.log calls:** 1
**Calls needing improvement:** 1
#### Change 1: [Line 68]
**Current:**
```typescript
Logger.log(
"error",
entity,
"activity",
logAction,
{ message: customError.getMessage() },
metadata,
);
```
**Current logAction derivation:**
```typescript
const logAction = snakeCase(customError?.name) ?? "unknown_error_thrown";
```
**Issue:** The logAction is dynamically generated from error name (e.g., "BadRequestError" becomes "bad_request_error"). While this creates consistent snake_case naming, it results in generic action names that don't describe the actual error context.
**Proposed:**
Keep the current implementation but document in code comments that these are intentionally generic since the specific error context is captured in the message field and error name is in metadata.
**Rationale:** The error service is a catch-all handler where the error type varies greatly. The current approach is actually reasonable here since:
1. The actual error details are in the message
2. The error type is preserved in customError.name
3. Changing this would require significant refactoring
4. The entity and URL already provide context
**Status:** NO CHANGE NEEDED - Current approach is acceptable for this use case.
---
### File: server/api/middlewares/handlePaymentMethodWebhook.ts
**Total Logger.log calls:** 2
**Calls needing improvement:** 2
#### Change 1: [Line 18]
**Current:**
```typescript
Logger.log(
"info",
"billing",
"activity",
"webhook_processing",
{
message: `Processing payment_method.attached for payment method ${data.id}, customer ${data.customer}`,
stripe_payment_method_id: data.id,
stripe_customer_id: data.customer as string,
},
{
type,
},
);
```
**Proposed:**
```typescript
Logger.log(
"info",
"billing",
"activity",
"process_payment_method_attached_webhook",
{
message: `Processing Stripe payment_method.attached webhook`,
stripe_payment_method_id: data.id,
stripe_customer_id: data.customer as string,
},
{
webhook_type: type,
},
);
```
**Rationale:** "webhook_processing" is too generic - many webhooks get processed. The proposed name specifies which webhook type. Renamed metadata key to "webhook_type" for clarity.
#### Change 2: [Line 31]
**Current:**
```typescript
Logger.log("info", "billing", "activity", "webhook_processed", {
message: `Successfully processed payment_method.attached for ${data.id}`,
stripe_payment_method_id: data.id,
});
```
**Proposed:**
```typescript
Logger.log("info", "billing", "activity", "payment_method_attached_webhook_complete", {
message: `Successfully processed payment_method.attached webhook`,
stripe_payment_method_id: data.id,
});
```
**Rationale:** "webhook_processed" is too generic for searching/filtering in CloudWatch. The proposed name is specific to this webhook type and indicates completion.
---
### File: server/api/middlewares/stripe/validateStripeWebhook.ts
**Total Logger.log calls:** 1
**Calls needing improvement:** 1
#### Change 1: [Line 28]
**Current:**
```typescript
Logger.log(
"info",
"billing",
"activity",
"webhook_received",
{ message: `Received event of type: ${type}` },
{
event_type: type,
event_id: id,
object,
},
);
```
**Proposed:**
```typescript
Logger.log(
"info",
"billing",
"activity",
"validate_stripe_webhook_signature",
{ message: `Validated Stripe webhook signature for event type: ${type}` },
{
stripe_event_type: type,
stripe_event_id: id,
stripe_object_type: object?.object,
},
);
```
**Rationale:** "webhook_received" doesn't convey that this log happens AFTER signature validation. The proposed name clarifies this is validation success. Renamed metadata keys to be more specific. Changed "object" to "stripe_object_type" since logging the entire object can be huge.
---
### File: server/loaders/BullMQ/Queues/attachLoggersToQueue.ts
**Total Logger.log calls:** 2
**Calls needing improvement:** 0
**Status:** Calls are clear with specific action names "job_failed" and "queue_error" with good context in messages.
---
### File: server/loaders/BullMQ/Workers/attachLoggersToWorker.ts
**Total Logger.log calls:** 4
**Calls needing improvement:** 0
**Status:** All calls are clear with specific action names like "worker_job_not_found", "worker_job_failed", "worker_job_stalled", and "worker_error".
---
### File: server/api/middlewares/lock/index.ts
**Total Logger.log calls:** 6
**Calls needing improvement:** 2
#### Change 1: [Line 74]
**Current:**
```typescript
Logger.log("info", "system", "activity", "lock_acquired", {
message: `Lock acquired: ${lockKey}`,
});
```
**Proposed:**
```typescript
Logger.log("info", "system", "activity", "distributed_lock_acquired", {
message: `Distributed lock acquired for ${lockKey}`,
},
{
lock_key: lockKey,
ttl_seconds: ttl,
});
```
**Rationale:** "lock_acquired" could be any type of lock. The proposed name clarifies this is a distributed (Redis) lock. Added metadata for filtering and debugging. Lock key is important context.
#### Change 2: [Line 140]
**Current:**
```typescript
Logger.log("error", "system", "activity", "lock_middleware_error", {
message: `Lock middleware error for resource ${resource}`,
});
```
**Proposed:**
```typescript
Logger.log("error", "system", "activity", "distributed_lock_acquisition_failed", {
message: `Failed to acquire distributed lock for resource ${resource}`,
},
{
resource,
error: error instanceof Error ? error.message : String(error),
stack: error instanceof Error ? error.stack : undefined,
});
```
**Rationale:** "lock_middleware_error" doesn't specify what failed. The proposed name clarifies the lock acquisition failed. Added error details to metadata.
---
### File: server/loaders/kysely.ts
**Total Logger.log calls:** 4
**Calls needing improvement:** 0
**Status:** All calls are clear with specific action names like "database_logging_error" (with clear context) and "database_closed".
---
### File: server/loaders/elasticSearch.ts
**Total Logger.log calls:** 1
**Calls needing improvement:** 1
#### Change 1: [Line 44]
**Current:**
```typescript
Logger.log(
"error",
"system",
"activity",
"elastic_search_load_error",
{ message: "Failed to load ElasticSearch indices" },
{
error: e instanceof Error ? e.message : String(e),
stack: e instanceof Error ? e.stack : undefined,
},
);
```
**Proposed:**
```typescript
Logger.log(
"error",
"system",
"activity",
"elasticsearch_index_initialization_error",
{ message: "Failed to initialize ElasticSearch indices during application startup" },
{
error: e instanceof Error ? e.message : String(e),
stack: e instanceof Error ? e.stack : undefined,
},
);
```
**Rationale:** "elastic_search_load_error" is vague. The proposed name clarifies this is index initialization error during startup.
---
### File: server/loaders/express.ts
**Total Logger.log calls:** 1
**Calls needing improvement:** 1
#### Change 1: [Line 86]
**Current:**
```typescript
Logger.log(
"error",
"system",
"activity",
"express_error",
{ message: err.message || "Unknown error" },
{
stack: err.stack,
error_name: err.name,
status_code: err.statusCode,
},
);
```
**Proposed:**
```typescript
Logger.log(
"error",
"system",
"activity",
"express_error_handler_caught_error",
{ message: err.message || "Unknown error caught by Express error middleware" },
{
stack: err.stack,
error_name: err.name,
status_code: err.statusCode,
url: req.originalUrl,
method: req.method,
},
);
```
**Rationale:** "express_error" is generic. The proposed name clarifies this is from the error handler middleware. Added URL and method context to metadata for debugging.
---
### File: server/app.ts
**Total Logger.log calls:** 4
**Calls needing improvement:** 0
**Status:** All calls are clear with specific action names like "unhandled_rejection", "server_not_found", "server_start_error", and "server_started".
---
### File: server/config/index.ts
**Total Logger.log calls:** 1
**Calls needing improvement:** 1
#### Change 1: [Line 105]
**Current:**
```typescript
Logger.log(
"error",
"system",
"activity",
"zod_error",
{
message: "Config schema parsing failed",
},
{
stack: parsedConfigResult.error.stack,
},
);
```
**Proposed:**
```typescript
Logger.log(
"error",
"system",
"activity",
"config_validation_error",
{
message: "Environment variable configuration failed Zod schema validation",
},
{
validation_errors: parsedConfigResult.error.errors,
stack: parsedConfigResult.error.stack,
},
);
```
**Rationale:** "zod_error" is too technical - focuses on tool not problem. The proposed name clarifies this is config validation failure. Added validation_errors to metadata to see which fields failed.
---
## Files With No Changes Needed
- server/loaders/BullMQ/Workers/attachLoggersToWorker.ts - All 4 calls already clear
- server/loaders/BullMQ/Queues/attachLoggersToQueue.ts - All 2 calls already clear
- server/loaders/BullMQ/Queues/ElasticSearch/jobs/Jobs/CloseExpiredJobs.ts - 1 call already clear
- server/loaders/BullMQ/Queues/ElasticSearch/jobs/Jobs/UnboostJobs.ts - 1 call already clear
- server/loaders/BullMQ/Queues/ElasticSearch/jobs/Tradesmen/SynchronizeTradesmen.ts - 1 call already clear
- server/loaders/BullMQ/Queues/ElasticSearch/events/Tradesmen/onExternalTradesmanSpreadsheetProcessed.ts - 1 call already clear
- server/loaders/BullMQ/Queues/index.ts - 1 call already clear
- server/loaders/BullMQ/Workers/index.ts - 1 call already clear
- server/loaders/kysely.ts - All 4 calls already clear (database_logging_error and database_closed are specific)
- server/services/activity/tradesman.ts - 1 call already clear
- server/services/application/index.ts - 1 call already clear (submit_application is specific)
- server/services/email/klaviyo.ts - All 4 calls already clear
- server/services/email/mailgun.ts - All 2 calls already clear
- server/services/email/nodeMailer.ts - All 2 calls already clear
- server/services/monetization/subscription/webhook.ts - All calls already clear (on_invoice_payment_failed, on_invoice_paid, on_subscription_deleted, on_invoice_upcoming)
- server/api/controllers/webhook/stripe/handleInvoiceWebhook.ts - All calls already clear (invoice_paid, invoice_paid_success, invoice_upcoming, etc.)
- server/api/controllers/webhook/stripe/handlePaymentIntentWebhook.ts - All calls already clear (payment_intent_succeeded, payment_intent_failed)
- server/api/controllers/webhook/stripe/handleSubscriptionWebhook.ts - All calls already clear (subscription_created, subscription_updated, subscription_deleted)
- server/services/auth.ts - All 3 calls already clear (sign_in, sign_up, forgot_password, reset_password, swap_roles)
- server/app.ts - All 4 calls already clear (unhandled_rejection, server_not_found, server_start_error, server_started)
- server/api/middlewares/logMiddleware.ts - Both calls already clear (HTTP method as action, log_middleware_error)
---
## Summary Statistics
### By Domain:
- ElasticSearch Domain: 15 changes
- BullMQ/Cron Jobs: 11 changes
- Billing/Stripe Webhooks: 5 changes
- Application Domain: 2 changes
- Infrastructure/System: 5 changes
- Email Services: 0 changes (already clear)
- Authentication: 0 changes (already clear)
### By Improvement Type:
- Action name improvements: 22 instances
- Message improvements: 18 instances
- Metadata key improvements: 8 instances
### Most Common Issues:
1. **Generic "get_" and "handle_" prefixes** (12 instances) - These don't convey enough information about the operation's purpose
2. **Reused action names across different contexts** (8 instances) - Like "incomplete_profile" used in multiple places with different meanings
3. **Missing context in messages** (10 instances) - Messages that don't explain what happened or why
4. **Vague metadata keys** (8 instances) - Keys like "data", "jobs", "tradesmen" that could be more specific like "eligible_jobs", "failed_applications"
### Patterns for Improvement:
1. **Action names should be specific and unique** - "fetch_jobs_for_automatic_apply" vs "get_jobs"
2. **Messages should answer "what happened?"** - Include counts, IDs, and outcomes
3. **Metadata keys should describe their content** - "eligible_tradesmen" vs "tradesmen"
4. **Error logs should include context** - What operation failed, not just "error"
5. **Similar operations should have consistent naming** - All webhook handlers follow same pattern
### Recommendations:
1. Establish naming conventions for common patterns (fetch_, validate_, process_, synchronize_)
2. Avoid reusing generic names like "error", "success", "handle", "process" without context
3. Include primary keys (job_id, tradesman_id, company_id) in message fields for CloudWatch Insights queries
4. Use descriptive metadata keys that indicate content type and purpose
5. For paired operations (start/complete), use consistent suffixes (_start, _complete)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment