-
Notifications
You must be signed in to change notification settings - Fork 4
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
chore(mock-server): make ui printer optional #165
Changes from 7 commits
2c4c368
7585492
c527c77
944ec3c
56d4635
25f2b3d
324bd0d
558321b
8ef747b
a2ee356
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 |
---|---|---|
@@ -1,3 +1,3 @@ | ||
# UI traces folder | ||
|
||
this folder is to place all the traces data we want to plt in the web ui. | ||
this folder is to place all the traces data we want to plot in the web ui. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,7 @@ | |
const dashdash = require('dashdash'); | ||
|
||
const luggite = require('./luggite'); | ||
const {JSONPrinter, InspectPrinter} = require('./printers'); | ||
const {JSONPrinter, InspectPrinter, FilePrinter} = require('./printers'); | ||
const {TraceWaterfallPrinter} = require('./waterfall'); | ||
const {MetricsSummaryPrinter} = require('./metrics-summary'); | ||
const {LogsSummaryPrinter} = require('./logs-summary'); | ||
|
@@ -50,6 +50,8 @@ const PRINTER_NAMES = [ | |
'metrics-summary', | ||
'logs-summary', | ||
'summary', | ||
|
||
'trace-file', // saving into fs for UI and other processing | ||
]; | ||
|
||
// This adds a custom cli option type to dashdash, to support `-o json,waterfall` | ||
|
@@ -97,6 +99,11 @@ const OPTIONS = [ | |
type: 'string', | ||
help: `The hostname on which servers should listen, by default this is "${DEFAULT_HOSTNAME}".`, | ||
}, | ||
{ | ||
names: ['ui'], | ||
type: 'bool', | ||
help: `Start a web server to inspect traces with some charts.`, | ||
}, | ||
]; | ||
|
||
async function main() { | ||
|
@@ -121,9 +128,15 @@ async function main() { | |
process.exit(0); | ||
} | ||
|
||
/** @type {Array<'http'|'grpc'|'ui'>} */ | ||
const services = ['http', 'grpc']; | ||
if (opts.ui) { | ||
services.push('ui'); | ||
} | ||
|
||
const otlpServer = new MockOtlpServer({ | ||
log, | ||
services: ['http', 'grpc', 'ui'], | ||
services, | ||
grpcHostname: opts.hostname || DEFAULT_HOSTNAME, | ||
httpHostname: opts.hostname || DEFAULT_HOSTNAME, | ||
uiHostname: opts.hostname || DEFAULT_HOSTNAME, | ||
|
@@ -186,6 +199,10 @@ async function main() { | |
case 'logs-summary': | ||
printers.push(new LogsSummaryPrinter(log)); | ||
break; | ||
|
||
case 'trace-file': | ||
printers.push(new FilePrinter(log)); | ||
break; | ||
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. Should the 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. One may want to open the UI just to inspect traces from past sessions but I think you question points to the case someone starting the UI but forgetting to put the printer in place, therefore not being able to se traces in the new traces UI. I think is as good point, I'll add it in the next commit |
||
} | ||
}); | ||
printers.forEach((p) => p.subscribe()); | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -26,6 +26,9 @@ | |||||
* printer.subscribe(); | ||||||
*/ | ||||||
|
||||||
const fs = require('fs'); | ||||||
const path = require('path'); | ||||||
|
||||||
const { | ||||||
diagchSub, | ||||||
CH_OTLP_V1_LOGS, | ||||||
|
@@ -153,8 +156,63 @@ class JSONPrinter extends Printer { | |||||
} | ||||||
} | ||||||
|
||||||
/** | ||||||
* This printer converts to a possible JSON representation of each service | ||||||
* request and saves it to a file. **Warning**: Converting OTLP service requests to JSON is fraught. | ||||||
*/ | ||||||
class FilePrinter extends Printer { | ||||||
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. note for reviewer: detaching the printer from the UI service makes sense. A possible scenario is to store traces for further analysis with other tools. Also changing the usage of |
||||||
constructor( | ||||||
log, | ||||||
indent, | ||||||
signals = ['trace'], | ||||||
dbDir = path.resolve(__dirname, '../db') | ||||||
) { | ||||||
super(log); | ||||||
this._indent = indent || 0; | ||||||
this._signals = signals; | ||||||
this._dbDir = dbDir; | ||||||
} | ||||||
printTrace(trace) { | ||||||
if (!this._signals.includes('trace')) return; | ||||||
const str = jsonStringifyTrace(trace, { | ||||||
indent: this._indent, | ||||||
normAttributes: true, | ||||||
}); | ||||||
const normTrace = JSON.parse(str); | ||||||
const tracesMap = new Map(); | ||||||
normTrace.resourceSpans.forEach((resSpan) => { | ||||||
resSpan.scopeSpans.forEach((scopeSpan) => { | ||||||
scopeSpan.spans.forEach((span) => { | ||||||
let traceSpans = tracesMap.get(span.traceId); | ||||||
|
||||||
if (!traceSpans) { | ||||||
traceSpans = []; | ||||||
tracesMap.set(span.traceId, traceSpans); | ||||||
} | ||||||
traceSpans.push(span); | ||||||
}); | ||||||
}); | ||||||
}); | ||||||
|
||||||
// Group als spans from the same trace into an ndjson file | ||||||
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. nit:
Suggested change
|
||||||
for (const [traceId, traceSpans] of tracesMap.entries()) { | ||||||
const filePath = path.join(this._dbDir, `trace-${traceId}.ndjson`); | ||||||
const stream = fs.createWriteStream(filePath, { | ||||||
flags: 'a', | ||||||
encoding: 'utf-8', | ||||||
}); | ||||||
|
||||||
for (const span of traceSpans) { | ||||||
stream.write(JSON.stringify(span) + '\n'); | ||||||
} | ||||||
stream.close(); | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
module.exports = { | ||||||
Printer, | ||||||
JSONPrinter, | ||||||
InspectPrinter, | ||||||
FilePrinter, | ||||||
}; |
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.
nit: don't need backtick string here... though I guess lint doesn't check that?
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.
it doesn't