-
Notifications
You must be signed in to change notification settings - Fork 101
Implemented consistent logging patterns #442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| # TeachLink Logging Strategy | ||
|
|
||
| ## Overview | ||
|
|
||
| This document defines the standard logging patterns, formats, and levels across the TeachLink ecosystem. Consistent, structured logging is critical for observability, debugging, and security auditing for both off-chain infrastructure (e.g., the Indexer) and on-chain smart contracts. | ||
|
|
||
| ## 1. Log Levels | ||
|
|
||
| The following standard log levels MUST be used across all off-chain services: | ||
|
|
||
| - **`error`**: System failures, critical errors requiring immediate attention, uncaught exceptions, and transaction failures. | ||
| - **`warn`**: Non-critical errors, anomalous but recoverable states, deprecated API usage, and retries. | ||
| - **`info`**: Normal system operations, business logic milestones (e.g., indexer block processing), startup/shutdown events. | ||
| - **`debug`**: Detailed diagnostic information, granular state transitions, payload dumps, and tracing for local development. | ||
|
|
||
| *Note: The environment variable `LOG_LEVEL` dictates the minimum severity level output by the application.* | ||
|
|
||
| ## 2. Structured Logging Format | ||
|
|
||
| All off-chain services MUST output logs in **Structured JSON format** when running in production (`NODE_ENV=production`). This ensures seamless ingestion by modern log aggregators and monitoring tools (e.g., Datadog, ELK, CloudWatch). | ||
|
|
||
| ### Standard JSON Schema | ||
|
|
||
| - **`timestamp`**: ISO 8601 UTC string (e.g., `2023-11-20T12:00:00.000Z`) | ||
| - **`level`**: The log severity level (e.g., `info`, `error`) | ||
| - **`context`**: The module, class, or domain emitting the log (e.g., `EventProcessorService`) | ||
| - **`message`**: A concise, human-readable description of the event | ||
| - **`data`**: (Optional) Additional contextual structured metadata (e.g., `txHash`, `userId`, `contractId`) | ||
|
|
||
| ### Example Production Log Entry | ||
|
|
||
| ```json | ||
| { | ||
| "timestamp": "2023-11-20T12:00:05.123Z", | ||
| "level": "info", | ||
| "context": "HorizonService", | ||
| "message": "Successfully processed bridge transaction", | ||
| "data": { | ||
| "txHash": "0xabc123...", | ||
| "ledger": 456789 | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| In non-production environments, logs may fallback to a human-readable, color-coded console output for better developer experience (DX). | ||
|
|
||
| ## 3. Smart Contract Logging | ||
|
|
||
| Soroban smart contracts operate natively on-chain, where traditional `stdout` logging is not viable for production. | ||
|
|
||
| 1. **Production Auditing (Events):** Use Soroban `#[contractevent]` structs as the primary mechanism for emitting structured logs. Events inherently provide structured, tamper-evident logging that off-chain indexers consume. | ||
| 2. **Local Debugging:** Use `env.logs().print(...)` exclusively for local testing and development. These statements should ideally be removed or disabled in production deployments. | ||
|
|
||
| ## 4. Best Practices | ||
|
|
||
| - **Do not log sensitive data:** PII (Personally Identifiable Information), private keys, secrets, and auth tokens MUST NOT be logged. | ||
| - **Include correlation IDs:** When processing user requests or multi-step asynchronous tasks, include a correlation identifier in the `data` field to trace the execution path. | ||
| - **Keep messages static:** The `message` field should be a static string (e.g., `"User login failed"`), while dynamic variables should be placed in the `data` object to facilitate aggregations and log metrics. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import { Module, Global } from '@nestjs/common'; | ||
| import { ConfigModule } from '@nestjs/config'; | ||
| import { LoggerService } from './logger.service'; | ||
|
|
||
|
Comment on lines
+1
to
+4
|
||
| @Global() | ||
| @Module({ | ||
| imports: [ConfigModule], | ||
| providers: [LoggerService], | ||
| exports: [LoggerService], | ||
| }) | ||
| export class LoggerModule {} | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,101 @@ | ||||||||||||||||||||||||||||||||
| import { LoggerService as NestLoggerService, Injectable, Scope } from '@nestjs/common'; | ||||||||||||||||||||||||||||||||
| import { ConfigService } from '@nestjs/config'; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| @Injectable({ scope: Scope.TRANSIENT }) | ||||||||||||||||||||||||||||||||
| export class LoggerService implements NestLoggerService { | ||||||||||||||||||||||||||||||||
| private context?: string; | ||||||||||||||||||||||||||||||||
|
Comment on lines
+4
to
+6
|
||||||||||||||||||||||||||||||||
| private readonly isProduction: boolean; | ||||||||||||||||||||||||||||||||
| private readonly logLevel: string; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| constructor(private configService: ConfigService) { | ||||||||||||||||||||||||||||||||
| this.isProduction = this.configService.get<string>('app.nodeEnv') === 'production'; | ||||||||||||||||||||||||||||||||
| this.logLevel = this.configService.get<string>('app.logLevel') || 'info'; | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| setContext(context: string) { | ||||||||||||||||||||||||||||||||
| this.context = context; | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| private shouldLog(level: string): boolean { | ||||||||||||||||||||||||||||||||
| const levels = ['debug', 'info', 'warn', 'error']; | ||||||||||||||||||||||||||||||||
| const configuredIndex = levels.indexOf(this.logLevel.toLowerCase()); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
| const configuredIndex = levels.indexOf(this.logLevel.toLowerCase()); | |
| const normalizedLogLevel = this.logLevel.toLowerCase(); | |
| const effectiveLogLevel = levels.includes(normalizedLogLevel) ? normalizedLogLevel : 'info'; | |
| const configuredIndex = levels.indexOf(effectiveLogLevel); |
Copilot
AI
Apr 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatMessage() assumes the trailing string in optionalParams is always a Nest context and pops it. For error(message, trace) calls (a common Nest signature), the stack trace is a string and will be incorrectly treated as the context, losing the trace and corrupting the context field. Handle error() separately (accept trace?: string, context?: string), or only treat the last string as context when there are 2+ trailing strings / a known context is provided.
| // Extract trailing string arguments (usually used as the context internally by NestJS) | |
| if (typeof optionalParams[optionalParams.length - 1] === 'string') { | |
| // Only treat the final string as context when there are multiple trailing strings. | |
| // This preserves Nest's common error(message, trace) signature, where the single | |
| // trailing string is a stack trace rather than a context value. | |
| let trailingStringCount = 0; | |
| for (let i = optionalParams.length - 1; i >= 0; i--) { | |
| if (typeof optionalParams[i] === 'string') { | |
| trailingStringCount++; | |
| } else { | |
| break; | |
| } | |
| } | |
| if (trailingStringCount >= 2) { |
Copilot
AI
Apr 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSON.stringify(message) can throw (e.g., circular references, BigInt) and would cause the logger to throw during normal operation. Wrap serialization in a try/catch and fall back to a safe serializer so logging is best-effort and never crashes the process.
Copilot
AI
Apr 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSON.stringify(metadata, null, 2) can also throw on non-serializable values (circular refs, BigInt). Since this is in the log path, ensure formatting cannot throw—use a safe serializer/fallback for dev output too.
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -30,53 +30,17 @@ export class ExplanationGenerator { | |||
| similarContent?: Array<[string, number]>, | ||||
| similarUsers?: string[] | ||||
| ): Types.RecommendationExplanation { | ||||
| let primaryReason = ''; | ||||
| const supportingSignals: string[] = []; | ||||
| const featureAttribution: Array<{ | ||||
| feature: string; | ||||
| importance: number; | ||||
| contribution: string; | ||||
| }> = []; | ||||
|
|
||||
| // Determine dominant signal | ||||
| const signals = Object.entries(rankingSignal); | ||||
| const [dominantSignal] = signals.reduce((a, b) => (a[1] > b[1] ? a : b)); | ||||
|
|
||||
| // Generate primary reason and supporting signals | ||||
| if (dominantSignal === 'collaborativeSignal' && similarUsers && similarUsers.length > 0) { | ||||
| primaryReason = `Users like you enjoyed this content`; | ||||
| supportingSignals.push(`Liked by ${similarUsers.length} similar learners`); | ||||
| featureAttribution.push({ | ||||
| feature: 'user_similarity', | ||||
| importance: rankingSignal.collaborativeSignal, | ||||
| contribution: `Based on similar learning patterns`, | ||||
| }); | ||||
| } else if (dominantSignal === 'contentSignal') { | ||||
| primaryReason = `Matches your interests`; | ||||
| const topics = Array.from(userProfile.features.topicAffinities.keys()).slice(0, 2); | ||||
| supportingSignals.push(`Related to your interest in ${topics.join(' and ')}`); | ||||
| featureAttribution.push({ | ||||
| feature: 'topic_match', | ||||
| importance: rankingSignal.contentSignal, | ||||
| contribution: `Content topic alignment with your profile`, | ||||
| }); | ||||
| } else if (dominantSignal === 'learningPathSignal') { | ||||
| primaryReason = `Recommended based on your learning path`; | ||||
| supportingSignals.push(`Prerequisite for your next goal`); | ||||
| featureAttribution.push({ | ||||
| feature: 'learning_path_fit', | ||||
| importance: rankingSignal.learningPathSignal, | ||||
| contribution: `Aligns with recommended progression`, | ||||
| }); | ||||
| } else if (dominantSignal === 'qualitySignal') { | ||||
| primaryReason = `High-quality content`; | ||||
| supportingSignals.push(`Highly rated by other learners`); | ||||
| featureAttribution.push({ | ||||
| feature: 'content_quality', | ||||
| importance: rankingSignal.qualitySignal, | ||||
| contribution: `Strong engagement and completion metrics`, | ||||
| }); | ||||
| } | ||||
| const { primaryReason, supportingSignals, featureAttribution } = this.extractDominantSignalExplanation( | ||||
| dominantSignal, | ||||
| rankingSignal, | ||||
| userProfile, | ||||
| similarUsers | ||||
|
Comment on lines
+38
to
+42
|
||||
| ); | ||||
|
|
||||
| // Add modality preference explanation | ||||
| if (userProfile.features.preferredModality === Types.ContentModality.VIDEO) { | ||||
|
|
@@ -105,20 +69,88 @@ export class ExplanationGenerator { | |||
| }; | ||||
| } | ||||
|
|
||||
| /** | ||||
| * Extracts the explanation based on the dominant ranking signal | ||||
| */ | ||||
| private extractDominantSignalExplanation( | ||||
| dominantSignal: string, | ||||
| rankingSignal: any, | ||||
| userProfile: Types.UserProfile, | ||||
| similarUsers?: string[] | ||||
|
Comment on lines
+75
to
+79
|
||||
| ): { | ||||
| primaryReason: string; | ||||
| supportingSignals: string[]; | ||||
| featureAttribution: Array<{ feature: string; importance: number; contribution: string }>; | ||||
| } { | ||||
| const result = { | ||||
| primaryReason: '', | ||||
| supportingSignals: [] as string[], | ||||
| featureAttribution: [] as Array<{ feature: string; importance: number; contribution: string }>, | ||||
| }; | ||||
|
|
||||
| switch (dominantSignal) { | ||||
| case 'collaborativeSignal': | ||||
| if (!similarUsers || similarUsers.length === 0) break; // Guard clause | ||||
| result.primaryReason = `Users like you enjoyed this content`; | ||||
| result.supportingSignals.push(`Liked by ${similarUsers.length} similar learners`); | ||||
| result.featureAttribution.push({ | ||||
| feature: 'user_similarity', | ||||
| importance: rankingSignal.collaborativeSignal, | ||||
| contribution: `Based on similar learning patterns`, | ||||
| }); | ||||
| break; | ||||
| case 'contentSignal': | ||||
| result.primaryReason = `Matches your interests`; | ||||
| const topics = Array.from(userProfile.features.topicAffinities.keys()).slice(0, 2); | ||||
| result.supportingSignals.push(`Related to your interest in ${topics.join(' and ')}`); | ||||
| result.featureAttribution.push({ | ||||
| feature: 'topic_match', | ||||
| importance: rankingSignal.contentSignal, | ||||
| contribution: `Content topic alignment with your profile`, | ||||
| }); | ||||
| break; | ||||
| case 'learningPathSignal': | ||||
| result.primaryReason = `Recommended based on your learning path`; | ||||
| result.supportingSignals.push(`Prerequisite for your next goal`); | ||||
| result.featureAttribution.push({ | ||||
| feature: 'learning_path_fit', | ||||
| importance: rankingSignal.learningPathSignal, | ||||
| contribution: `Aligns with recommended progression`, | ||||
| }); | ||||
| break; | ||||
| case 'qualitySignal': | ||||
| result.primaryReason = `High-quality content`; | ||||
| result.supportingSignals.push(`Highly rated by other learners`); | ||||
| result.featureAttribution.push({ | ||||
| feature: 'content_quality', | ||||
| importance: rankingSignal.qualitySignal, | ||||
| contribution: `Strong engagement and completion metrics`, | ||||
| }); | ||||
| break; | ||||
| } | ||||
|
|
||||
| return result; | ||||
| } | ||||
|
|
||||
| /** | ||||
| * Rule-based explanation generator | ||||
| */ | ||||
| private generateRuleBasedExplanation( | ||||
| userProfile: Types.UserProfile, | ||||
| signals: string[] | ||||
| ): string { | ||||
| if (!userProfile) return ''; // Guard clause | ||||
|
|
||||
|
Comment on lines
+142
to
+143
|
||||
| if (!userProfile) return ''; // Guard clause |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This strategy doc states that all off-chain services MUST emit structured JSON logs in production, but the PR doesn’t actually integrate the provided LoggerService into any service (e.g., the Nest indexer still uses
@nestjs/commonLogger). Either update the documentation to describe the current state, or wire the implementation into the runtime services so the doc is accurate.