-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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(unstable): replace SpanExporter with TracerProvider #27473
Changes from all commits
0effa32
025755b
16687f2
6f5883b
4b29c10
1abb71c
5d43c08
c60b57e
4a56938
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 |
---|---|---|
|
@@ -43,10 +43,7 @@ const { | |
Uint8Array, | ||
Promise, | ||
} = primordials; | ||
const { | ||
getAsyncContext, | ||
setAsyncContext, | ||
} = core; | ||
const { getAsyncContext, setAsyncContext } = core; | ||
|
||
import { InnerBody } from "ext:deno_fetch/22_body.js"; | ||
import { Event } from "ext:deno_web/02_event.js"; | ||
|
@@ -90,9 +87,8 @@ import { | |
import { hasTlsKeyPairOptions, listenTls } from "ext:deno_net/02_tls.js"; | ||
import { SymbolAsyncDispose } from "ext:deno_web/00_infra.js"; | ||
import { | ||
endSpan, | ||
builtinTracer, | ||
enterSpan, | ||
Span, | ||
TRACING_ENABLED, | ||
} from "ext:deno_telemetry/telemetry.ts"; | ||
import { | ||
|
@@ -288,28 +284,28 @@ class InnerRequest { | |
|
||
// * is valid for OPTIONS | ||
if (path === "*") { | ||
return this.#urlValue = "*"; | ||
return (this.#urlValue = "*"); | ||
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. Surprised that dprint is not removing these changes to parentheses 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. I accidentally ran prettier - it added these parentheses |
||
} | ||
|
||
// If the path is empty, return the authority (valid for CONNECT) | ||
if (path == "") { | ||
return this.#urlValue = this.#methodAndUri[1]; | ||
return (this.#urlValue = this.#methodAndUri[1]); | ||
} | ||
|
||
// CONNECT requires an authority | ||
if (this.#methodAndUri[0] == "CONNECT") { | ||
return this.#urlValue = this.#methodAndUri[1]; | ||
return (this.#urlValue = this.#methodAndUri[1]); | ||
} | ||
|
||
const hostname = this.#methodAndUri[1]; | ||
if (hostname) { | ||
// Construct a URL from the scheme, the hostname, and the path | ||
return this.#urlValue = this.#context.scheme + hostname + path; | ||
return (this.#urlValue = this.#context.scheme + hostname + path); | ||
} | ||
|
||
// Construct a URL from the scheme, the fallback hostname, and the path | ||
return this.#urlValue = this.#context.scheme + this.#context.fallbackHost + | ||
path; | ||
return (this.#urlValue = this.#context.scheme + this.#context.fallbackHost + | ||
path); | ||
} | ||
|
||
get completed() { | ||
|
@@ -396,10 +392,7 @@ class InnerRequest { | |
return; | ||
} | ||
|
||
PromisePrototypeThen( | ||
op_http_request_on_cancel(this.#external), | ||
callback, | ||
); | ||
PromisePrototypeThen(op_http_request_on_cancel(this.#external), callback); | ||
} | ||
} | ||
|
||
|
@@ -503,12 +496,7 @@ function fastSyncResponseOrStream( | |
autoClose = true; | ||
} | ||
PromisePrototypeThen( | ||
op_http_set_response_body_resource( | ||
req, | ||
rid, | ||
autoClose, | ||
status, | ||
), | ||
op_http_set_response_body_resource(req, rid, autoClose, status), | ||
(success) => { | ||
innerRequest?.close(success); | ||
op_http_close_after_finish(req); | ||
|
@@ -538,10 +526,7 @@ function mapToCallback(context, callback, onError) { | |
updateSpanFromRequest(span, request); | ||
} | ||
|
||
response = await callback( | ||
request, | ||
new ServeHandlerInfo(innerRequest), | ||
); | ||
response = await callback(request, new ServeHandlerInfo(innerRequest)); | ||
|
||
// Throwing Error if the handler return value is not a Response class | ||
if (!ObjectPrototypeIsPrototypeOf(ResponsePrototype, response)) { | ||
|
@@ -618,12 +603,12 @@ function mapToCallback(context, callback, onError) { | |
mapped = function (req, _span) { | ||
const oldCtx = getAsyncContext(); | ||
setAsyncContext(context.asyncContext); | ||
const span = new Span("deno.serve", { kind: 1 }); | ||
const span = builtinTracer().startSpan("deno.serve", { kind: 1 }); | ||
try { | ||
enterSpan(span); | ||
return SafePromisePrototypeFinally( | ||
origMapped(req, span), | ||
() => endSpan(span), | ||
() => span.end(), | ||
); | ||
} finally { | ||
// equiv to exitSpan. | ||
|
@@ -670,7 +655,7 @@ function formatHostName(hostname: string): string { | |
// because browsers in Windows don't resolve "0.0.0.0". | ||
// See the discussion in https://github.com/denoland/deno_std/issues/1165 | ||
if ( | ||
(Deno.build.os === "windows") && | ||
Deno.build.os === "windows" && | ||
(hostname == "0.0.0.0" || hostname == "::") | ||
) { | ||
return "localhost"; | ||
|
@@ -712,11 +697,12 @@ function serve(arg1, arg2) { | |
const wantsHttps = hasTlsKeyPairOptions(options); | ||
const wantsUnix = ObjectHasOwn(options, "path"); | ||
const signal = options.signal; | ||
const onError = options.onError ?? function (error) { | ||
// deno-lint-ignore no-console | ||
console.error(error); | ||
return internalServerError(); | ||
}; | ||
const onError = options.onError ?? | ||
function (error) { | ||
// deno-lint-ignore no-console | ||
console.error(error); | ||
return internalServerError(); | ||
}; | ||
Comment on lines
+700
to
+705
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. Lots of unrelated changes due to moving code around :D 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. Sorry |
||
|
||
if (wantsUnix) { | ||
const listener = listen({ | ||
|
@@ -825,10 +811,7 @@ function serveHttpOn(context, addr, callback) { | |
const promiseErrorHandler = (error) => { | ||
// Abnormal exit | ||
// deno-lint-ignore no-console | ||
console.error( | ||
"Terminating Deno.serve loop due to unexpected error", | ||
error, | ||
); | ||
console.error("Terminating Deno.serve loop due to unexpected error", error); | ||
context.close(); | ||
}; | ||
|
||
|
@@ -946,7 +929,7 @@ function registerDeclarativeServer(exports) { | |
port: servePort, | ||
hostname: serveHost, | ||
[kLoadBalanced]: (serveIsMain && serveWorkerCount > 1) || | ||
(serveWorkerCount !== null), | ||
serveWorkerCount !== null, | ||
onListen: ({ port, hostname }) => { | ||
if (serveIsMain) { | ||
const nThreads = serveWorkerCount > 1 | ||
|
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.
Not important for this PR, but could we use some sort of enum object for the kind? The numerical value is not super informative.
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 is an enum in the user API. I'll see about adding it to the internal api too