Skip to content

judge: enhance tracing configuration #1112

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.

Sign up for GitHub

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

Open
renbaoshuo wants to merge 1 commit into
base: master
Choose a base branch
from
Open

judge: enhance tracing configuration #1112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/hydrojudge/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
"@hydrooj/common": "workspace:^",
"@hydrooj/utils": "workspace:^",
"@opentelemetry/api": "^1.9.0",
"@opentelemetry/exporter-trace-otlp-grpc": "^0.208.0",
"@opentelemetry/exporter-trace-otlp-http": "^0.208.0",
"@opentelemetry/exporter-trace-otlp-proto": "^0.208.0",
"@opentelemetry/sdk-node": "^0.208.0",
"@zip.js/zip.js": "^2.8.11",
"cac": "^6.7.14",
Expand Down
6 changes: 6 additions & 0 deletions packages/hydrojudge/src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,14 @@ export const JudgeSettings = Schema.object({
secret: Schema.string().description('Judge Token Secret').default(randomstring(32)),
disable: Schema.boolean().description('Disable builtin judge').default(false),
tracing: Schema.object({
exporter: Schema.union([
Schema.const('grpc'),
Schema.const('http'),
Schema.const('proto'),
]).description('Tracing exporter').default('http'),
endpoint: Schema.string().role('url').description('Tempo endpoint').default('http://localhost:4318'),
samplePercentage: Schema.number().description('Sample percentage').default(0).min(0).max(1),
attributes: Schema.any().description('Tracing attributes').default({}),
}),
detail: Schema.union([
Schema.const('full'),
Expand Down
9 changes: 8 additions & 1 deletion packages/hydrojudge/src/daemon.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,14 @@ async function daemon() {
const shouldRun = await versionCheck((msg) => log.error(msg));
if (!shouldRun) process.exit(1);
const tracing = getConfig('tracing');
if (tracing?.endpoint && tracing?.samplePercentage) initTracing(tracing.endpoint, tracing.samplePercentage);
if (tracing?.endpoint && tracing?.samplePercentage) {
initTracing({
exporter: tracing.exporter,
endpoint: tracing.endpoint,
samplePercentage: tracing.samplePercentage,
attributes: tracing.attributes,
});
}
const _hosts = getConfig('hosts');
const queue = new PQueue({ concurrency: Infinity });
await fs.ensureDir(getConfig('tmp_dir'));
Expand Down
7 changes: 6 additions & 1 deletion packages/hydrojudge/src/hosts/builtin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,12 @@ export async function apply(ctx: HydroContext) {
const tracing = getConfig('tracing');
if (tracing?.endpoint && tracing?.samplePercentage) {
ctx.effect(() => {
const sdk = initTracing(tracing.endpoint, tracing.samplePercentage);
const sdk = initTracing({
exporter: tracing.exporter,
endpoint: tracing.endpoint,
samplePercentage: tracing.samplePercentage,
attributes: tracing.attributes,
});
return () => sdk.shutdown();
});
}
Expand Down
30 changes: 26 additions & 4 deletions packages/hydrojudge/src/tracing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,39 @@
import { OTLPTraceExporter as OTLPTraceExporterGrpc } from '@opentelemetry/exporter-trace-otlp-grpc';
import { OTLPTraceExporter as OTLPTraceExporterHttp } from '@opentelemetry/exporter-trace-otlp-http';
import { OTLPTraceExporter as OTLPTraceExporterProto } from '@opentelemetry/exporter-trace-otlp-proto';
import { NodeSDK, resources, tracing } from '@opentelemetry/sdk-node';
import { Logger } from './log';

const logger = new Logger('tracing');

export function initTracing(endpoint: string, samplePercentage = 1.0) {
const traceExporter = new OTLPTraceExporterHttp({
url: endpoint.includes('/v1/traces') ? endpoint : `${endpoint}/v1/traces`,
});
interface TracingConfig {
exporter?: 'grpc' | 'http' | 'proto';
endpoint: string;
samplePercentage?: number;
attributes?: Record<string, any>;
}

export function initTracing(config: TracingConfig) {
const { endpoint, samplePercentage = 1.0, attributes = {}, exporter = 'http' } = config;
let traceExporter;
if (exporter === 'grpc') {
traceExporter = new OTLPTraceExporterGrpc({
url: endpoint,
});
} else if (exporter === 'proto') {
traceExporter = new OTLPTraceExporterProto({
url: endpoint,
});
} else {
traceExporter = new OTLPTraceExporterHttp({
url: endpoint.includes('/v1/traces') ? endpoint : `${endpoint}/v1/traces`,
});
}
Comment on lines +23 to +31
Copy link

@coderabbitai coderabbitai bot Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

OpenTelemetry OTLP trace exporter proto endpoint URL format /v1/traces

💡 Result:

Short answer:

  • The OTLP/HTTP trace export endpoint path is /v1/traces (POST of ExportTraceServiceRequest). If you set OTEL_EXPORTER_OTLP_ENDPOINT the SDK appends v1/traces to that base URL; if you set OTEL_EXPORTER_OTLP_TRACES_ENDPOINT the URL is used as‑is. [1][2]

Examples:

References:
[1] OpenTelemetry Protocol Exporter (OTLP/HTTP endpoint behavior).
[2] OTLP specification: default HTTP path /v1/traces (ExportTraceServiceRequest).

🌐 Web query:

@opentelemetry/exporter-trace-otlp-proto JavaScript endpoint configuration

💡 Result:

Short answer — you can set the endpoint either in the exporter constructor (collectorOptions.url) or with OTEL environment variables.

How:

  • In code (HTTP/proto exporter): pass url in the config when creating OTLPTraceExporter:
    const { OTLPTraceExporter } = require('@opentelemetry/exporter-trace-otlp-proto');
    const exporter = new OTLPTraceExporter({ url: 'http://collector:4318/v1/traces', headers: { /* optional */ } });
    (constructor accepts an OTLPExporterNodeConfigBase). [1]

  • With env vars (preferred for deployment): set OTEL_EXPORTER_OTLP_ENDPOINT for a base URL or OTEL_EXPORTER_OTLP_TRACES_ENDPOINT for a signal-specific URL. When using OTEL_EXPORTER_OTLP_ENDPOINT with HTTP, the SDK will append /v1/traces for traces; per-signal vars are used as-is. Defaults are http://localhost:4318 (HTTP) and http://localhost:4317 (gRPC). [2][3]

Notes:

  • For HTTP/proto endpoints include the /v1/traces path (unless you use the base env var and let the SDK append it). [2][3]
  • For gRPC exporters don’t append the /v1/* path — use the appropriate gRPC endpoint (e.g., http(s) scheme indicates secure/insecure per spec). [3]

Sources:
[1] OTLPTraceExporter constructor docs.
[2] OpenTelemetry SDK OTLP exporter configuration (env vars & defaults).
[3] OpenTelemetry Protocol Exporter spec (per-signal URL rules).

🌐 Web query:

OTLPTraceExporterProto vs OTLPTraceExporterHttp JavaScript @opentelemetry endpoint path behavior

💡 Result:

Short answer — behavior differs on whether the exporter will append the signal path (/v1/traces) for you:

  • @opentelemetry/exporter-trace-otlp-http (OTLPTraceExporterHttp)

    • When you give a base OTLP endpoint (e.g. via OTEL_EXPORTER_OTLP_ENDPOINT or the exporter option url without the signal path) the HTTP exporter will construct the per-signal path and expects/uses /v1/traces for traces. If you set OTEL_EXPORTER_OTLP_ENDPOINT it appends /v1/traces; if you set OTEL_EXPORTER_OTLP_TRACES_ENDPOINT you must include the full /v1/traces. (open-telemetry.github.io)

  • @opentelemetry/exporter-trace-otlp-proto (OTLPTraceExporterProto)

    • The proto exporter’s documented defaults and examples show the exporter’s url already including the full signal path (default http://localhost:4318/v1/traces). Per the OTLP exporter specification, per-signal endpoints can be used “as‑is” (the implementation must honor the URL path component). In practice supply the full /v1/traces path for the proto exporter (or use the per-signal env var). (opentelemetry.io)

Practical rule:

  • If you pass a base endpoint (no path) and rely on OTEL_EXPORTER_OTLP_ENDPOINT, the HTTP exporter will append /v1/traces for you; for proto exporters treat the url as signal-specific (include /v1/traces) or use the per-signal env var. (open-telemetry.github.io)

If you want, tell me the exact code or env var you’re using and I’ll confirm the correct url to set.

Citations:

🌐 Web query:

OTLPTraceExporterProto JavaScript automatically append v1 traces path endpoint

💡 Result:

Yes — the OTLP trace HTTP exporters in OpenTelemetry JS will append the /v1/traces path automatically when you set the generic OTEL_EXPORTER_OTLP_ENDPOINT (or when you omit url and use the default). If you want to supply a full per-signal endpoint you must include the path yourself (OTEL_EXPORTER_OTLP_TRACES_ENDPOINT or url option). [1][2]

Examples:

Sources:
[1] OpenTelemetry JS OTLP Trace exporter docs (env var per-signal behavior).
[2] OpenTelemetry docs / exporter examples showing default url and automatic /v1/traces behavior.

🏁 Script executed:

head -25 packages/hydrojudge/src/tracing.ts

Repository: hydro-dev/Hydro

Length of output: 1081

Proto exporter must include /v1/traces path like the HTTP exporter.

The OTLPTraceExporterProto uses the same OTLP/HTTP endpoint path as OTLPTraceExporterHttp, but the current code passes the endpoint directly without ensuring the /v1/traces path is appended. If a user provides a base endpoint like http://localhost:4318, the proto exporter will send requests to the wrong path and fail to export traces.

Suggested fix:

     } else if (exporter === 'proto') {
         traceExporter = new OTLPTraceExporterProto({
-            url: endpoint,
+            url: endpoint.includes('/v1/traces') ? endpoint : `${endpoint}/v1/traces`,
         });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} else if (exporter === 'proto') {
traceExporter = new OTLPTraceExporterProto({
url: endpoint,
});
} else {
traceExporter = new OTLPTraceExporterHttp({
url: endpoint.includes('/v1/traces') ? endpoint : `${endpoint}/v1/traces`,
});
}
} else if (exporter === 'proto') {
traceExporter = new OTLPTraceExporterProto({
url: endpoint.includes('/v1/traces') ? endpoint : `${endpoint}/v1/traces`,
});
} else {
traceExporter = new OTLPTraceExporterHttp({
url: endpoint.includes('/v1/traces') ? endpoint : `${endpoint}/v1/traces`,
});
}
🤖 Prompt for AI Agents 
In packages/hydrojudge/src/tracing.ts around lines 23 to 31, the proto exporter
is created with the raw endpoint which may be a base URL (e.g.
http://localhost:4318) and therefore misses the required /v1/traces path; update
the OTLPTraceExporterProto instantiation to mirror the HTTP branch by checking
if endpoint includes '/v1/traces' and, if not, append '/v1/traces' before
passing it to the constructor so the proto exporter sends traces to the correct
path.

const sdk = new NodeSDK({
resource: resources.resourceFromAttributes({
'service.name': 'hydrojudge',
'service.version': require('../package.json').version,
...attributes,
}),
traceExporter,
sampler: new tracing.TraceIdRatioBasedSampler(samplePercentage),
Expand Down
Loading