-
Notifications
You must be signed in to change notification settings - Fork 4
feat: setup logger #282
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
feat: setup logger #282
Changes from 8 commits
c6f2675
5050af7
d26a737
a1597a2
c5ad8db
fced977
fa9e9d9
e335f94
8289202
a191899
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,50 @@ | ||||||||||||||||||
networks: | ||||||||||||||||||
loki: | ||||||||||||||||||
|
||||||||||||||||||
services: | ||||||||||||||||||
loki: | ||||||||||||||||||
image: grafana/loki:latest | ||||||||||||||||||
ports: | ||||||||||||||||||
- "3100:3100" | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Limit public exposure of Loki/Grafana for local stacks Bind to localhost to avoid exposing UIs and APIs on all interfaces. - - "3100:3100"
+ - "127.0.0.1:3100:3100"
...
- - "3000:3000"
+ - "127.0.0.1:3000:3000" Also applies to: 47-48 🤖 Prompt for AI Agents
|
||||||||||||||||||
command: -config.file=/etc/loki/local-config.yaml | ||||||||||||||||||
networks: | ||||||||||||||||||
- loki | ||||||||||||||||||
|
||||||||||||||||||
Comment on lines
+5
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add persistence and restart policies Currently all data (Loki indexes/chunks, Grafana dashboards) are ephemeral and services won't restart on failure.
loki:
image: grafana/loki:latest
+ restart: unless-stopped
+ volumes:
+ - loki-data:/loki
...
grafana:
+ restart: unless-stopped
+ volumes:
+ - grafana-data:/var/lib/grafana
networks:
loki:
services:
...
+volumes:
+ loki-data:
+ grafana-data: Optionally add healthchecks once validated tools exist in the images. Also applies to: 21-50 🤖 Prompt for AI Agents
|
||||||||||||||||||
promtail: | ||||||||||||||||||
image: grafana/promtail:latest | ||||||||||||||||||
volumes: | ||||||||||||||||||
- /var/log:/var/log | ||||||||||||||||||
command: -config.file=/etc/promtail/config.yml | ||||||||||||||||||
networks: | ||||||||||||||||||
- loki | ||||||||||||||||||
|
||||||||||||||||||
grafana: | ||||||||||||||||||
environment: | ||||||||||||||||||
- GF_PATHS_PROVISIONING=/etc/grafana/provisioning | ||||||||||||||||||
- GF_AUTH_ANONYMOUS_ENABLED=true | ||||||||||||||||||
- GF_AUTH_ANONYMOUS_ORG_ROLE=Admin | ||||||||||||||||||
- GF_FEATURE_TOGGLES_ENABLE=alertingSimplifiedRouting,alertingQueryAndExpressionsStepMode | ||||||||||||||||||
Comment on lines
+24
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Anonymous admin in Grafana is unsafe by default GF_AUTH_ANONYMOUS_ENABLED=true with GF_AUTH_ANONYMOUS_ORG_ROLE=Admin grants full admin to anyone who can hit port 3000. For anything beyond strictly local, disable anonymous auth or reduce role to Viewer and set admin creds. Suggested hardening:
- - GF_AUTH_ANONYMOUS_ENABLED=true
- - GF_AUTH_ANONYMOUS_ORG_ROLE=Admin
+ - GF_AUTH_ANONYMOUS_ENABLED=false
+ # For local dev with anonymous viewer:
+ # - GF_AUTH_ANONYMOUS_ENABLED=true
+ # - GF_AUTH_ANONYMOUS_ORG_ROLE=Viewer 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||
entrypoint: | ||||||||||||||||||
- sh | ||||||||||||||||||
- -euc | ||||||||||||||||||
- | | ||||||||||||||||||
mkdir -p /etc/grafana/provisioning/datasources | ||||||||||||||||||
cat <<EOF > /etc/grafana/provisioning/datasources/ds.yaml | ||||||||||||||||||
apiVersion: 1 | ||||||||||||||||||
datasources: | ||||||||||||||||||
- name: Loki | ||||||||||||||||||
type: loki | ||||||||||||||||||
access: proxy | ||||||||||||||||||
orgId: 1 | ||||||||||||||||||
url: http://loki:3100 | ||||||||||||||||||
basicAuth: false | ||||||||||||||||||
isDefault: true | ||||||||||||||||||
version: 1 | ||||||||||||||||||
editable: false | ||||||||||||||||||
EOF | ||||||||||||||||||
/run.sh | ||||||||||||||||||
image: grafana/grafana:latest | ||||||||||||||||||
ports: | ||||||||||||||||||
- "3000:3000" | ||||||||||||||||||
networks: | ||||||||||||||||||
- loki |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
data/* |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,8 @@ | |
"axios": "^1.6.7", | ||
"evault-core": "workspace:*", | ||
"graphql-request": "^6.1.0", | ||
"pino": "^9.8.0", | ||
"pino-loki": "^2.6.0", | ||
Comment on lines
+21
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainLogging deps look good; clean up test config drift and stray dependency pino/pino-loki additions are appropriate. However:
Please consolidate on a single test runner and remove unused packages/config. Suggested clean-up (if standardizing on Vitest): "dependencies": {
@@
- "test": "^3.3.0",
+ // removed: "test" dependency appears unintended
},
"devDependencies": {
- "@types/jest": "^29.5.0",
- "jest": "^29.5.0",
- "ts-jest": "^29.1.0",
+ // removed Jest-related packages; using Vitest
},
- "jest": {
- "preset": "ts-jest",
- "testEnvironment": "node",
- "testMatch": [
- "**/__tests__/**/*.test.ts"
- ]
- }
+ // removed Jest config; using Vitest Optional verification: 🏁 Script executed: #!/bin/bash
# Confirm Jest is unused and find stray 'test' dependency usage
rg -n --no-heading 'from .*jest|describe\\(|it\\(|test\\(|@jest' -g '!node_modules'
jq -r '.dependencies.test // empty' infrastructure/web3-adapter/package.json Length of output: 302 I’ll correct the regex and inspect both code usage and package.json scripts/deps: 🏁 Script executed: #!/bin/bash
set -e
echo "Searching code for any Jest usage patterns..."
rg -n --no-heading \
-e 'from .*jest' \
-e 'describe\(' \
-e 'it\(' \
-e 'test\(' \
-e '@jest' \
--glob '*.{ts,tsx,js,jsx}'
echo
echo "Current scripts in package.json:"
jq .scripts infrastructure/web3-adapter/package.json
echo
echo "Relevant deps in package.json:"
jq '{ dependencies: .dependencies, devDependencies: .devDependencies }' infrastructure/web3-adapter/package.json | \
jq '{ testDependency: .dependencies.test, vitest: .devDependencies.vitest, jest: .devDependencies.jest, tsJest: .devDependencies["ts-jest"], typesJest: .devDependencies["@types/jest"] }' Length of output: 19836 Consolidate on Vitest: remove Jest, stray “test” dep, and add Vitest All code is wired for Vitest (see
After these changes, run 🤖 Prompt for AI Agents
|
||
"sqlite3": "^5.1.7", | ||
"test": "^3.3.0", | ||
"uuid": "^11.1.0", | ||
|
@@ -35,6 +37,8 @@ | |
"jest": { | ||
"preset": "ts-jest", | ||
"testEnvironment": "node", | ||
"testMatch": ["**/__tests__/**/*.test.ts"] | ||
"testMatch": [ | ||
"**/__tests__/**/*.test.ts" | ||
] | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export * from "./transport"; | ||
export * from "./logger"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import pino from "pino"; | ||
import { transport } from "./transport"; | ||
|
||
export const logger = pino(transport); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent ❓ Verification inconclusiveConfigure log level and redact sensitive fields Initialize Pino with explicit level and redaction for safety; keep your existing Loki transport. -export const logger = pino(transport);
+export const logger = pino(
+ {
+ level: process.env.LOG_LEVEL || "info",
+ // Redact commonly sensitive keys; extend as needed
+ redact: {
+ paths: ["password", "token", "authorization", "auth", "*.password", "*.token", "*.authorization"],
+ remove: true,
+ },
+ },
+ transport,
+); Note: ensure non-dev deployments do not rely on default Loki basicAuth credentials; set LOKI_USERNAME/LOKI_PASSWORD securely. Configure Pino logger with default level and redact sensitive fields To ensure consistent log verbosity and prevent accidental leakage of secrets, initialize Pino with an explicit
Apply this diff: -export const logger = pino(transport);
+export const logger = pino(
+ {
+ level: process.env.LOG_LEVEL || "info",
+ // Redact commonly sensitive keys; extend as needed
+ redact: {
+ paths: [
+ "password",
+ "token",
+ "authorization",
+ "auth",
+ "*.password",
+ "*.token",
+ "*.authorization"
+ ],
+ remove: true,
+ },
+ },
+ transport,
+); Also verify that non-dev deployments supply 🤖 Prompt for AI Agents
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,16 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { transport as pinoTransport } from "pino"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import type { LokiOptions } from "pino-loki"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const transport = pinoTransport<LokiOptions>({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
target: "pino-loki", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
options: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
host: process.env.LOKI_URL || "http://localhost:3100", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
labels: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
app: "web3-adapter", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
basicAuth: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
username: process.env.LOKI_USERNAME || "admin", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
password: process.env.LOKI_PASSWORD || "admin", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+4
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainFix TypeScript usage of pino.transport and harden auth defaults
Apply: -import type { LokiOptions } from "pino-loki";
+import type { LokiOptions } from "pino-loki";
-export const transport = pinoTransport<LokiOptions>({
- target: "pino-loki",
- options: {
- host: process.env.LOKI_URL || "http://localhost:3100",
- labels: { app: "web3-adapter" },
- basicAuth: {
- username: process.env.LOKI_USERNAME || "admin",
- password: process.env.LOKI_PASSWORD || "admin",
- },
- },
-});
+const lokiOptions: LokiOptions = {
+ host: process.env.LOKI_URL || "http://localhost:3100",
+ labels: { app: "web3-adapter" },
+ basicAuth: {
+ username: process.env.LOKI_USERNAME || "admin",
+ password: process.env.LOKI_PASSWORD || "admin",
+ },
+};
+export const transport = pinoTransport({
+ target: "pino-loki",
+ options: lokiOptions,
+}); Optional: dev-only default credentials const isDev = process.env.NODE_ENV !== "production";
const lokiUser = process.env.LOKI_USERNAME ?? (isDev ? "admin" : undefined);
const lokiPass = process.env.LOKI_PASSWORD ?? (isDev ? "admin" : undefined);
// throw if missing in prod
if (!isDev && (!lokiUser || !lokiPass)) {
throw new Error("LOKI_USERNAME/LOKI_PASSWORD are required in production");
} Please verify that control-panel.compose.yml provides LOKI_URL/creds to this service. 🏁 Script executed: #!/bin/bash
# Check for LOKI_* env wiring and where logger is consumed
rg -n --no-heading 'LOKI_URL|LOKI_USERNAME|LOKI_PASSWORD|logger\.info|pino-loki' Length of output: 1709 Fix pinoTransport usage and lock down credentials
Apply in infrastructure/web3-adapter/src/logging/transport.ts: -import type { LokiOptions } from "pino-loki";
-
-export const transport = pinoTransport<LokiOptions>({
- target: "pino-loki",
- options: {
- host: process.env.LOKI_URL || "http://localhost:3100",
- labels: { app: "web3-adapter" },
- basicAuth: {
- username: process.env.LOKI_USERNAME || "admin",
- password: process.env.LOKI_PASSWORD || "admin",
- },
- },
-});
+import type { LokiOptions } from "pino-loki";
+
+// build and validate our Loki options
+const lokiOptions: LokiOptions = {
+ host: process.env.LOKI_URL || "http://localhost:3100",
+ labels: { app: "web3-adapter" },
+ basicAuth: {
+ // in prod these must be set, only default in dev
+ username: process.env.LOKI_USERNAME ?? (process.env.NODE_ENV !== "production" ? "admin" : undefined),
+ password: process.env.LOKI_PASSWORD ?? (process.env.NODE_ENV !== "production" ? "admin" : undefined),
+ },
+};
+
+// enforce credentials in production
+if (process.env.NODE_ENV === "production" && (!lokiOptions.basicAuth.username || !lokiOptions.basicAuth.password)) {
+ throw new Error("LOKI_USERNAME and LOKI_PASSWORD are required in production");
+}
+
+export const transport = pinoTransport({
+ target: "pino-loki",
+ options: lokiOptions,
+}); 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -12,6 +12,7 @@ import { PostgresSubscriber } from "../web3adapter/watchers/subscriber"; | |||
|
||||
config({ path: path.resolve(__dirname, "../../../../.env") }); | ||||
|
||||
console.log("SAODIHUOPIFHDSA", process.env.PICTIQUE_DATABASE_URL) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove sensitive database URL logging Printing the full PICTIQUE_DATABASE_URL can leak credentials to logs (stdout, CI, Loki/Grafana). Please remove this line: -console.log("SAODIHUOPIFHDSA", process.env.PICTIQUE_DATABASE_URL) If you need a sanity check, log a non-sensitive indicator instead (e.g., presence or a masked DSN host). I can add a small utility to safely mask DSNs if you prefer. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||
export const AppDataSource = new DataSource({ | ||||
type: "postgres", | ||||
url: process.env.PICTIQUE_DATABASE_URL, | ||||
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -16,7 +16,7 @@ import { adapter } from "./web3adapter/watchers/subscriber"; | |||||||
config({ path: path.resolve(__dirname, "../../../.env") }); | ||||||||
|
||||||||
const app = express(); | ||||||||
const port = process.env.PORT || 3000; | ||||||||
const port = process.env.PORT || 1111; | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainCoerce PORT to a number and verify infra/docs for new default 1111 Passing a string PORT to app.listen can be error-prone. Also, changing the fallback port can affect local/dev setups if PORT isn’t set. Apply: -const port = process.env.PORT || 1111;
+const port = Number(process.env.PORT) || 1111; Please verify:
🏁 Script executed: #!/bin/bash
# Find places that assume 3000 or reference pictique-api port
rg -n --no-heading -A2 -B2 $'pictique-api|:3000|PORT=3000' Length of output: 3743 Coerce PORT to a number in pictique-api We didn’t find any Docker/compose files, scripts or docs referencing port 3000 for pictique-api, so bumping the default to 1111 has no downstream impact. Please apply the numeric coercion: – const port = process.env.PORT || 1111;
+ const port = Number(process.env.PORT) || 1111; 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||
|
||||||||
// Initialize database connection and adapter | ||||||||
AppDataSource.initialize() | ||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -24,7 +24,7 @@ const initializeDatabase = async () => { | |||||||||
await AppDataSource.initialize(); | ||||||||||
server.log.info("Database connection initialized"); | ||||||||||
} catch (error) { | ||||||||||
server.log.error("Error during database initialization:", error); | ||||||||||
server.log.error({message: "Error during database initialization", detail: error}); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Preserve error stack/metadata in logs With Fastify/Pino, pass the error directly or under Apply one of: -server.log.error({message: "Error during database initialization", detail: error});
+server.log.error(error as Error, "Error during database initialization"); or -server.log.error({message: "Error during database initialization", detail: error});
+server.log.error({ err: error }, "Error during database initialization"); Consider aligning other catch blocks to the same pattern for consistency. 📝 Committable suggestion
Suggested change
Suggested change
🤖 Prompt for AI Agents
|
||||||||||
process.exit(1); | ||||||||||
} | ||||||||||
}; | ||||||||||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
🛠️ Refactor suggestion
Pin images to specific versions or digests instead of latest
Using :latest is non-deterministic and can break deployments unexpectedly.
Please adjust tags to your validated versions.
Also applies to: 14-14, 46-46
🤖 Prompt for AI Agents