Image Render: Support Tracing #586

lucychen-grafana · 2024-12-10T17:13:05Z

Testing Instructions:

Test as service:

Install tempo https://grafana.com/docs/tempo/latest/getting-started/docker-example/

Run the dockerfile for tempo local storage in the local directory

Checkout this branch of grafana and run grafana-enterprise

Set otlp address in defaults.ini

[tracing.opentelemetry.otlp]
address = localhost:4317

Checkout this pr grafana-image-renderer branch. Run as remote server
Open Localhost:3000; run a report or PDF creation, or any call to the image renderer service
Check traces: Explore -> Select ‘gdev-tempo’ datasource -> run query type Search with resource.service.name="grafana-image-renderer"

If there is no Tempo datasource, create a new one with url set to http://localhost:3200
Select the TraceID related to the IR GET request and look into the spans

Check logs in the console to confirm trace_id and span_id is included in the logs

To test the plugin on Mac:

Build the plugin locally for Mac:
1. Update your Makefile to set ARCH = darwin-arm64-unknown
2. Run make build_package
3. The plugin will be built in the dist folder
Copy / paste the built plugin to your Grafana plugins folder and rename it to grafana-image-renderer
In your Grafana .ini file:
1. Comment your remote renderer settings in the [rendering] section
2. Add the following config to enable traces and allow running the IR plugin unsigned:

[plugin.grafana-image-renderer]
rendering_tracing_url = http://localhost:4318/v1/traces

[plugins]
allow_loading_unsigned_plugins = grafana-image-renderer

Additional Pr to review:
Deployment_tools pr for config setup
Grafana pr

CLAassistant · 2024-12-10T17:13:12Z

All committers have signed the CLA.

src/logger.ts

lucychen-grafana · 2025-01-30T16:44:50Z

src/tracing.ts

@@ -0,0 +1,41 @@
+import {NodeSDK} from '@opentelemetry/sdk-node';


I followed OpenTelemetry Doc for Node.js to implement this

src/tracing.ts

lucychen-grafana · 2025-01-30T16:55:02Z

src/tracing.ts

+export  function startTracing(log: Logger) {
+    sdk.start();
+    log.info('Starting tracing');
+
+    process.on('SIGTERM', () => {
+        sdk.shutdown()
+            .then(() => log.debug('Tracing terminated'))
+            .catch((error) => log.error('Error terminating tracing', error))
+            .finally(() => process.exit(0));
+    });


Create startTracing separately from the rest of initialization steps b/c we want to only capture traces if it's enabled

src/app.ts

AgnesToulet

This looks good! I tested it and this worked. However, I don't find the current traces very useful as there is a big blank gap in the middle.

An easy place to add some manual spans would be in the withTimingMetrics function. Something like that but adding some config checks:

  async withTimingMetrics<T>(step: string, callback: () => Promise<T>): Promise<T> {
    if (this.config.timingMetrics) {
      const endTimer = this.metrics.durationHistogram.startTimer({ step });
      const res = this.tracer.startActiveSpan(step, async (span) => {
        const cbRes = await callback();
        span.end();
        return cbRes;
      });
      endTimer();

      return res;
    } else {
      return callback();
    }
  }

The Browser class could initialize its tracer in its constructor (this should work for all browser implementations, e.g. with mode: "clustered"):

export class Browser {
  tracer: Tracer;

  constructor(protected config: RenderingConfig, protected log: Logger, protected metrics: Metrics) {
    this.log.debug('Browser initialized', 'config', this.config);
    this.tracer = trace.getTracer('browser');
  }
  ...

Also, I looked into what is missing to have a link between Grafana traces and image renderer traces and this simple change is working (to enable traces in Grafana, you need to set:

[tracing.opentelemetry.otlp]
# otlp destination (ex localhost:4317)
address = localhost:4317

). I will test it out a bit more and check if this need to be initialized only when tracing is enabled or if this is a no-op when tracing is disabled.

package.json

AgnesToulet · 2025-02-03T15:37:04Z

src/tracing.ts

@@ -0,0 +1,41 @@
+import {NodeSDK} from '@opentelemetry/sdk-node';


This whole file should be formatted using Prettier linter (we don't have a check for that in CI but we should do our best).

src/tracing.ts

Co-authored-by: Agnès Toulet <[email protected]>

lucychen-grafana · 2025-02-12T22:39:55Z

[tracing.opentelemetry.otlp]

otlp destination (ex localhost:4317)

address = localhost:4317

@AgnesToulet setting this address is enable traces in Grafana in dev env right? Traces should already be enabled in Cloud via Faro

AgnesToulet · 2025-02-14T14:12:07Z

Yes it's for your local environment. It's enabled by default on Cloud (Faro is for frontend monitoring, its configuration is in a different section of the ini file, I don't think both are related).

…fana-image-renderer into lucychen/add_tracing

* add tracing to plugin mode * update dev.json * update imports * add tracing in CSV endpoints * fix tests * fix and simplify config * fix test

AgnesToulet · 2025-02-21T14:35:47Z

README.md

@@ -40,6 +39,33 @@ This plugin is not compatible with the current Grafana Docker image and requires

 If you still want to install the plugin with the Grafana Docker image, refer to the instructions on building a custom Grafana image in [Grafana Docker documentation](https://grafana.com/docs/grafana/latest/setup-grafana/configure-docker/#build-a-custom-grafana-docker-image).

+### Plugin for Mac ARM64


I think this should be here instead: https://github.com/grafana/grafana-image-renderer/blob/master/docs/building_from_source.md

This README file is the one that will be published on grafana.com and should stay simple I think (I just noticed that the "testing" section should also probably be elsewhere).

We could link to the "Build from source" doc in this file though, to help with discoverability. Like, in the "Supported operating systems" section, you could add:

For Mac ARM64, you need to [build the plugin from source](https://github.com/grafana/grafana-image-renderer/blob/master/docs/building_from_source.md) or use the [remote rendering installation](https://github.com/grafana/grafana-image-renderer?tab=readme-ov-file#remote-rendering-service-installation).

Ok. I can move the instruction to Build from source doc and also create a new doc for testing instructions

AgnesToulet · 2025-02-21T14:36:17Z

README.md

@@ -73,6 +99,11 @@ The following example shows how you can run Grafana and the remote HTTP renderin
       ports:
         - 8081
   ```
+   _Notes:_


This command is already in the "build from source" section.

AgnesToulet · 2025-02-21T14:39:31Z

devenv/docker/custom-config/config.json

+    "dumpio": false,
+
+    "tracing": {
+      "url": "http://host.docker.internal:4318/v1/traces"


This works only on Windows and Mac, not Linux. I think the best would be to provide the whole setup so to copy/paste the content of the Tempo docker-compose file, to replace this with http://tempo:4318/v1/traces and to provision the Tempo datasource so people can easily set up tracing (I wonder if we should do all this in the custom-config folder or create a new tracing folder).

Oh i see. Maybe better to create separate tracing folder w tracing config.json and docker-compose file if end user don't need tracing enabled at all times

src/plugin/v2/grpc_plugin.ts

AgnesToulet · 2025-02-21T14:43:22Z

src/tracing.ts

+
+  return new NodeSDK({
+    resource: new Resource({
+      [SEMRESATTRS_SERVICE_NAME]: 'grafana-image-renderer',


As mentioned in a previous comment, I think it would be best to be able to configure this.

Configure the [SEMRESATTRS_SERVICE_NAME]? Or the whole resource?

The [SEMRESATTRS_SERVICE_NAME], I think internally, we'd prefer to use render-service for example. But grafana-image-renderer is a better default.

Why do we want to give that an option to configure it? If in the future we decide to rename it, then it'll make querying disjuncted and would have to take into account of both

It's not to configure it multiple times, it's just to configure it once when you set up tracing so that it matches your own deployment convention and make the traces easier to discover. The thing is that the value you put makes the most sense as the default but doesn't match what we use on our Cloud.

Co-authored-by: Agnès Toulet <[email protected]>

src/tracing.ts

devenv/docker/tracing/docker-compose.yaml

src/config/rendering.ts

AgnesToulet · 2025-02-25T13:03:39Z

src/config/rendering.ts

@@ -118,6 +132,11 @@ const envConfig: Record<Mode, Keys<RenderingConfig>> = {
    verboseLogging: 'GF_PLUGIN_RENDERING_VERBOSE_LOGGING',
    dumpio: 'GF_PLUGIN_RENDERING_DUMPIO',
    timingMetrics: 'GF_PLUGIN_RENDERING_TIMING_METRICS',
+    tracing: {
+      url: 'GF_PLUGIN_RENDERING_TRACING_URL',
+      serviceName: 'GF_PLUGIN_RENDERING_TRACING_SERVICE_NAME'


This is not used by the populateRenderingConfigFromEnv function. This should either be used or removed. I think we can remove it as it's less likely to be an issue for people using the image renderer in plugin mode (usually, this means no Kubernetes setup where you already gave a name at your services). And we can always add it later if requested.

Co-authored-by: Agnès Toulet <[email protected]>

nmarrs

Hm tried to test this locally but ran into this error in my grafana local server logs:

ERROR[03-06|07:42:50] OpenTelemetry handler returned an error  logger=tracing err="traces export: context deadline exceeded: rpc error: code = Unavailable desc = connection error: desc = \"transport: Error while dialing: dial tcp [::1]:4317: connect: connection refused\""

Honestly though if both you and Agnes were able to run this locally we should unblock merging this and we can make any tweaks as necessary once we start getting data from dev / ops instances

One thing I was a little confused on is whether or not traces should work for image renderer in plugin mode or just http server mode (I think the latter). Your directions in PR description include instructions for mac for running as plugin mode but I'm not sure traces will be sent in that case 🤔

nmarrs · 2025-03-06T06:55:12Z

src/config/rendering.ts

@@ -207,4 +224,8 @@ export function populateRenderingConfigFromEnv(config: RenderingConfig, env: Nod
  if (env[envKeys.timingMetrics!]) {
    config.timingMetrics = env[envKeys.timingMetrics!] === 'true';
  }
+
+  if (env[envKeys.tracing?.url!]) {


this is kinda ew haha (using the ! here as it can cause things to break if this is not defined in the object) - given this is used right now in other places and things aren't breaking its probably fine for the time being

nmarrs · 2025-03-06T06:57:42Z

src/service/http-server.integration.test.ts

@@ -95,15 +99,15 @@ function getGrafanaEndpoint(domain: string) {
 let envSettings = {
  saveDiff: false,
  updateGolden: false,
-}
+};


i'm guessing a lot of these extra changes are just running lint - maybe we should consider making a follow-up PR to image renderer to run lint across all of the files to clean this up some for the future

nmarrs · 2025-03-06T06:58:31Z

src/service/http-server.ts

@@ -330,4 +318,20 @@ export class HttpServer {
      return res.status(500).json({ error: e.message });
    }
  };
+
+  getHeaders(req: express.Request<any, any, any, RenderOptions, any>): HTTPHeaders {


can we not define the objects here in place of "any" type?

evictorero and others added 7 commits August 2, 2024 18:31

Add tracing

d974217

changed url ingestion and tried to extract traceparent

eb1db17

removing unused code

bb8a21a

remove unused packages and add logs

1a0ee72

add tracing info to logs and tracing configuration

c8ed844

Merge branch 'master' into poc-tracing

4ff2f6c

Merge remote-tracking branch 'origin/master' into lucychen/add_tracing

7f10f00

lucychen-grafana linked an issue Dec 10, 2024 that may be closed by this pull request

Render: Support tracing #164

Open

lucychen-grafana added 6 commits January 10, 2025 19:32

fix config override

36b1891

fix argv.config

9eaa8cf

fix trace initialization and starting

a741f58

remove custom log format

71445a8

update json

37cfaa7

change url to env

45f4968

lucychen-grafana commented Jan 30, 2025

View reviewed changes

src/logger.ts Show resolved Hide resolved

lucychen-grafana commented Jan 30, 2025

View reviewed changes

src/tracing.ts Show resolved Hide resolved

lucychen-grafana commented Jan 30, 2025

View reviewed changes

src/app.ts Show resolved Hide resolved

lucychen-grafana requested review from a team, juanicabanas and AgnesToulet and removed request for a team January 30, 2025 16:55

lucychen-grafana marked this pull request as ready for review January 30, 2025 16:56

Merge branch 'master' into lucychen/add_tracing

18e1d25

AgnesToulet reviewed Feb 3, 2025

View reviewed changes

lucychen-grafana and others added 3 commits February 4, 2025 03:29

Apply suggestions from code review

3f191e2

Co-authored-by: Agnès Toulet <[email protected]>

fix yarn file

1eca03d

prettier lint

575c323

AgnesToulet mentioned this pull request Feb 10, 2025

Poc tracing #562

Closed

add custom trace for browser

dcd3967

AgnesToulet and others added 9 commits February 14, 2025 15:44

Merge branch 'lucychen/add_tracing' of https://github.com/grafana/gra…

87f42d2

…fana-image-renderer into lucychen/add_tracing

propagate traces back to Grafana

987f0e7

Add tracing to plugin mode and improve config management (#602)

837f072

* add tracing to plugin mode * update dev.json * update imports * add tracing in CSV endpoints * fix tests * fix and simplify config * fix test

update config for docker

4d7f051

add to readme

8e94864

update readme

6bc6e9b

update readme

4842979

prettier

57d7d5e

update tracing

e2c0cef

lucychen-grafana mentioned this pull request Feb 21, 2025

Image Render: Propagate traces in HTTP mode grafana/grafana#100559

Open

3 tasks

AgnesToulet reviewed Feb 21, 2025

View reviewed changes

lucychen-grafana and others added 6 commits February 21, 2025 17:27

change config and docs

6cc75b5

Update src/plugin/v2/grpc_plugin.ts

05c54c0

Co-authored-by: Agnès Toulet <[email protected]>

create new docker-compose for tracing

eb68fe0

add serviceName to config

4d27607

Merge remote-tracking branch 'origin/master' into lucychen/add_tracing

c4c28e8

update yarn

16d5b7e

lucychen-grafana requested a review from AgnesToulet February 25, 2025 03:42

AgnesToulet reviewed Feb 25, 2025

View reviewed changes

lucychen-grafana and others added 2 commits February 25, 2025 10:52

Apply suggestions from code review

2813722

Co-authored-by: Agnès Toulet <[email protected]>

remove env servicename

d78747f

lucychen-grafana requested a review from evictorero February 26, 2025 19:21

nmarrs approved these changes Mar 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Image Render: Support Tracing #586

Image Render: Support Tracing #586

lucychen-grafana commented Dec 10, 2024 •

edited

Loading

CLAassistant commented Dec 10, 2024 •

edited

Loading

lucychen-grafana Jan 30, 2025

lucychen-grafana Jan 30, 2025

AgnesToulet left a comment

AgnesToulet Feb 3, 2025

lucychen-grafana commented Feb 12, 2025

otlp destination (ex localhost:4317)

AgnesToulet commented Feb 14, 2025

AgnesToulet Feb 21, 2025

lucychen-grafana Feb 21, 2025

AgnesToulet Feb 21, 2025

AgnesToulet Feb 21, 2025

lucychen-grafana Feb 21, 2025

AgnesToulet Feb 21, 2025

lucychen-grafana Feb 21, 2025

AgnesToulet Feb 21, 2025

lucychen-grafana Feb 22, 2025

AgnesToulet Feb 24, 2025

AgnesToulet Feb 25, 2025

nmarrs left a comment

nmarrs Mar 6, 2025

nmarrs Mar 6, 2025 •

edited

Loading

nmarrs Mar 6, 2025

		@@ -0,0 +1,41 @@
		import {NodeSDK} from '@opentelemetry/sdk-node';

		@@ -40,6 +39,33 @@ This plugin is not compatible with the current Grafana Docker image and requires

		If you still want to install the plugin with the Grafana Docker image, refer to the instructions on building a custom Grafana image in [Grafana Docker documentation](https://grafana.com/docs/grafana/latest/setup-grafana/configure-docker/#build-a-custom-grafana-docker-image).

		### Plugin for Mac ARM64

Image Render: Support Tracing #586

Are you sure you want to change the base?

Image Render: Support Tracing #586

Conversation

lucychen-grafana commented Dec 10, 2024 • edited Loading

CLAassistant commented Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AgnesToulet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucychen-grafana commented Feb 12, 2025

otlp destination (ex localhost:4317)

AgnesToulet commented Feb 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmarrs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmarrs Mar 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucychen-grafana commented Dec 10, 2024 •

edited

Loading

CLAassistant commented Dec 10, 2024 •

edited

Loading

nmarrs Mar 6, 2025 •

edited

Loading