fix(tracing): actually set the tracing options in the root context #5118

JGAntunes · 2023-07-03T17:05:01Z

🎉 Thanks for submitting a pull request! 🎉

Summary

Follow up to #5100. This time I actually tested the whole flow e2e:

https://ui.honeycomb.io/gatsbyinc/environments/buildbot-production/datasets/build.buildbot/result/4iBeKJnbMyg/trace/wouYttwcmUe?fields[]=c_name&fields[]=c_service.name&span=cd6c2b7921ee9376

For us to review and ship your PR efficiently, please perform the following steps:

Open a bug/issue before writing your code 🧑‍💻. This ensures
we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
something that`s on fire 🔥 (e.g. incident related), you can skip this step.
Read the contribution guidelines 📖. This ensures
your code follows our style guide and passes our tests.
Update or add tests (if any source code was changed or added) 🧪
Update or add documentation (if features were changed or added) 📝
Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

github-actions · 2023-07-04T10:50:14Z

This pull request adds or modifies JavaScript (.js, .cjs, .mjs) files.
Consider converting them to TypeScript.

JGAntunes · 2023-07-04T16:56:39Z

packages/build/src/tracing/main.ts

@@ -1,6 +1,5 @@
+import { HoneycombSDK } from '@honeycombio/opentelemetry-node'


The HoneycombSDK provides a couple of things out the box which make the initialisation simpler.

JGAntunes · 2023-07-04T16:57:39Z

packages/build/src/tracing/main.ts

@@ -43,7 +43,7 @@ export const startTracing = function (options: TracingOptions, logger: (...args:

  // Sets the current trace ID and span ID based on the options received
  // this is used as a way to propagate trace context from Buildbot
-  trace.setSpanContext(context.active(), {
+  return trace.setSpanContext(context.active(), {


This is the actual change that ensures the traceId and spanId are using when initialising the first span.

JGAntunes · 2023-07-04T16:58:31Z

packages/build/src/steps/run_step.ts

+  // If there's no `coreStepId` then this is a plugin execution
+  const spanName = `run-step-${coreStepId || 'plugin'}`


coreStepId only exists for internal steps. We replace this for plugin for plugin executions and we can use the package_name to aggregate on the package.

do you mind giving more information about the plugin vs steps?

For example in this comment, it mentions we have core steps, build commands or plugin. I understand the meaning of the steps we do in buildbot, but not in here. Maybe this is a question too broad, so we can catch up on "Making Moves"

JGAntunes · 2023-07-04T16:59:40Z

packages/build/src/core/normalize_flags.ts

+    // tracing.apiKey defaults to '-' else we'll get warning logs if not using
+    // honeycomb directly - https://github.com/honeycombio/honeycomb-opentelemetry-node/issues/201


Honeycomb emits a set of console.warn logs we can't skip if we don't provide an apiKey - honeycombio/honeycomb-opentelemetry-node#201. This is why for production we pass -, since we're going to use a collector.

JGAntunes · 2023-07-04T17:00:01Z

packages/build/src/tracing/main.ts

    serviceName: ROOT_PACKAGE_JSON.name,
-    traceExporter,
-    instrumentations: [new HttpInstrumentation()],
+    protocol: 'grpc',


Both in prod and locally we'll always use grpc

JGAntunes · 2023-07-04T17:05:51Z

packages/build/src/tracing/main.ts

 import { context, trace, propagation, SpanStatusCode, diag, DiagLogLevel, DiagLogger } from '@opentelemetry/api'
-import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-grpc'
-import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'


Couldn't manage to setup auto instrumentation. Seems like it's now working for esm but it's fairly recent and requires us to pass additional node flags:

feat: ESM support for instrumentation open-telemetry/opentelemetry-js#3698

https://github.com/open-telemetry/opentelemetry-js/blob/dccd90603bf52ba3b52eced9e8a6489bbdc78388/examples/esm-http-ts/package.json#L10

I've been thinking a bit about this and I think in the future we might want to have this tracing initialisation happen in a separate module we can use via node --require @netlify/tracing-initialisation ./call-neltify/build (or node --import once we move to node 20 I believe?) since it would:

Allow us to out of the box use the same initialisation process for any node process we startup in Buildbot

Make @netlify/build and other packages only depend on @opentelemetry/api (see fix(deps): update netlify packages cli#5826 (comment))

4xposed · 2023-07-05T10:09:26Z

packages/build/src/core/build.ts

@@ -37,9 +37,9 @@ export const startBuild = function (flags: Partial<BuildFlags>) {

  const { bugsnagKey, tracingOpts, debug, systemLogFile, ...flagsA } = normalizeFlags(flags, logs)
  const errorMonitor = startErrorMonitor({ flags: { tracingOpts, debug, systemLogFile, ...flagsA }, logs, bugsnagKey })
-  startTracing(tracingOpts, getSystemLogger(logs, debug, systemLogFile))
+  const rootTracingContext = startTracing(tracingOpts, getSystemLogger(logs, debug, systemLogFile))


~~would it be: rootTrace? is "Context" a thing in the JS/TS world?~~
scratch that, I just saw the context bit on the other file

Context in this case is specifc to the open-telemetry JS implementation. In this case we're returning a context from:

build/packages/build/src/tracing/main.ts

Lines 44 to 51 in edf7b83

// Sets the current trace ID and span ID based on the options received

// this is used as a way to propagate trace context from Buildbot

return trace.setSpanContext(context.active(), {

traceId: options.traceId,

spanId: options.parentSpanId,

traceFlags: options.traceFlags,

isRemote: true,

})

See:

https://github.com/netlify/build/pull/5118/files#r1252231543

Also:

https://opentelemetry.io/docs/instrumentation/js/context/

This context will hold the specific traceId, spanId, etc. That we pass over from Buildbot.

JGAntunes self-assigned this Jul 3, 2023

JGAntunes force-pushed the fix/trace-properties-handover branch from 1f4e0f0 to d5c81a2 Compare July 3, 2023 17:08

github-actions bot added the Adds or modifies js files label Jul 4, 2023

JGAntunes commented Jul 4, 2023

View reviewed changes

JGAntunes marked this pull request as ready for review July 4, 2023 17:12

JGAntunes requested review from a team as code owners July 4, 2023 17:12

JGAntunes added 9 commits July 4, 2023 18:12

fix(tracing): actually set the tracing options in the root context

cd44968

chore: try to skip the warning logs

8bc9de3

fix: don't pass over description

ed6ff40

chore: enable debug mode for tracing

7e44b07

fix: actually setup the protocol to grpc

18b3d77

fix: plugin names for spans

eb62165

chore: remove auto instrumentation

62cab00

chore: remove verbose logging

3fb3182

chore: add some docs

edf7b83

JGAntunes force-pushed the fix/trace-properties-handover branch from 750837c to edf7b83 Compare July 4, 2023 17:12

4xposed reviewed Jul 5, 2023

View reviewed changes

4xposed approved these changes Jul 5, 2023

View reviewed changes

JGAntunes added the automerge label Jul 5, 2023

kodiakhq bot added 3 commits July 5, 2023 10:20

Merge branch 'main' into fix/trace-properties-handover

51da7ef

Merge branch 'main' into fix/trace-properties-handover

2103437

Merge branch 'main' into fix/trace-properties-handover

705b93a

kodiakhq bot merged commit 6353314 into main Jul 5, 2023

kodiakhq bot deleted the fix/trace-properties-handover branch July 5, 2023 11:01

netlify-circleci mentioned this pull request Jul 5, 2023

chore: release main #5129

Merged

JGAntunes mentioned this pull request Jul 6, 2023

Setup a parent based sampler honeycombio/honeycomb-opentelemetry-node#203

Closed

netlify-circleci mentioned this pull request Jul 26, 2023

chore: release main #5208

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tracing): actually set the tracing options in the root context #5118

fix(tracing): actually set the tracing options in the root context #5118

JGAntunes commented Jul 3, 2023 •

edited

Loading

github-actions bot commented Jul 4, 2023

JGAntunes Jul 4, 2023

JGAntunes Jul 4, 2023

JGAntunes Jul 4, 2023

biruwon Jul 5, 2023

JGAntunes Jul 4, 2023

JGAntunes Jul 4, 2023

JGAntunes Jul 4, 2023

4xposed Jul 5, 2023 •

edited

Loading

JGAntunes Jul 5, 2023 •

edited

Loading

		@@ -1,6 +1,5 @@
		import { HoneycombSDK } from '@honeycombio/opentelemetry-node'

		// If there's no `coreStepId` then this is a plugin execution
		const spanName = `run-step-${coreStepId \|\| 'plugin'}`

		// tracing.apiKey defaults to '-' else we'll get warning logs if not using
		// honeycomb directly - https://github.com/honeycombio/honeycomb-opentelemetry-node/issues/201

	// Sets the current trace ID and span ID based on the options received
	// this is used as a way to propagate trace context from Buildbot
	return trace.setSpanContext(context.active(), {
	traceId: options.traceId,
	spanId: options.parentSpanId,
	traceFlags: options.traceFlags,
	isRemote: true,
	})

fix(tracing): actually set the tracing options in the root context #5118

fix(tracing): actually set the tracing options in the root context #5118

Conversation

JGAntunes commented Jul 3, 2023 • edited Loading

Summary

github-actions bot commented Jul 4, 2023

JGAntunes Jul 4, 2023

Choose a reason for hiding this comment

JGAntunes Jul 4, 2023

Choose a reason for hiding this comment

JGAntunes Jul 4, 2023

Choose a reason for hiding this comment

biruwon Jul 5, 2023

Choose a reason for hiding this comment

JGAntunes Jul 4, 2023

Choose a reason for hiding this comment

JGAntunes Jul 4, 2023

Choose a reason for hiding this comment

JGAntunes Jul 4, 2023

Choose a reason for hiding this comment

4xposed Jul 5, 2023 • edited Loading

Choose a reason for hiding this comment

JGAntunes Jul 5, 2023 • edited Loading

Choose a reason for hiding this comment

JGAntunes commented Jul 3, 2023 •

edited

Loading

4xposed Jul 5, 2023 •

edited

Loading

JGAntunes Jul 5, 2023 •

edited

Loading