Skip to content

Add tracing to plugin mode and improve config management #602

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

Jump to bottom
Merged
merged 7 commits into from
Feb 20, 2025
Merged

Add tracing to plugin mode and improve config management #602

merged 7 commits into from
Feb 20, 2025

Conversation

AgnesToulet
Copy link
Contributor

@AgnesToulet AgnesToulet commented Feb 18, 2025 •
edited by lucychen-grafana
Loading

This PR adds tracing to the plugin mode and update a few things from the original PR:

  • Read the config file before initializing tracing, this allows to have the exporter URL set in the config file instead of using an env variable
  • Do no intialize the tracer variable if tracing is disabled
  • Propagate the trace context back to Grafana

What remains:

  • Add tracing to the CSV endpoint
  • Compare traces between the original PR and this one to ensure we didn't lose anything by reading the config file before initializing tracing
  • Update Grafana upstream PR
  • Test, test and test again (in plugin mode, in remote server mode and in Docker)

To test the plugin on Mac:

  1. 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
  2. Copy / paste the built plugin to your Grafana plugins folder and rename it to grafana-image-renderer
  3. 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

@AgnesToulet AgnesToulet changed the title Add tracing to plugin mode Add tracing to plugin mode and improve config management Feb 18, 2025
lucychen-grafana
lucychen-grafana reviewed Feb 18, 2025
View reviewed changes
src/plugin/v2/grpc_plugin.ts

if (this.config.rendering.tracing.url) {
const output: TraceCarrier = {};
propagation.inject(context.active(), output);
Copy link
Contributor

Choose a reason for hiding this comment

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

You are referencing this manual propagationcorrect?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

lucychen-grafana
lucychen-grafana reviewed Feb 18, 2025
View reviewed changes
dev.json
"timingMetrics": true,

"tracing": {
"url": ""
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we set url to http://localhost:4318/v1/traces for dev.json automatically in dev

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could as this is not generating errors when you don't have Tempo set up. We should update the doc though to explain how to set up the environment to get traces. We could add this URL there as well.

lucychen-grafana
lucychen-grafana reviewed Feb 20, 2025
View reviewed changes
src/config/rendering.ts
@@ -118,6 +126,9 @@ 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',
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this linked to

[plugin.grafana-image-renderer]
rendering_tracing_url

? Where is it being set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the Grafana codebase, the config fields for plugins (so in sections starting with plugin.) are automatically converted to env variables to be used by plugins. They follow the syntax GF_PLUGIN_<config field in upper case>

@lucychen-grafana
Copy link
Contributor

I tested as a remote server and as a plugin and they look good.
I tried running it on Docker but my setup might not be correct...the logs are not showing traces the trace url is set in the custom-config config.json

@AgnesToulet
Copy link
Contributor Author

To make it work in Docker, you need to update the URL to http://host.docker.internal:4318/v1/traces. This allows Docker services to communicate if they are not set up in the same docker-compose file. Maybe we could update the custom-config setup to include traces (and setup the tempo services in the same docker-compose file instead of linking to the Tempo repo).

@lucychen-grafana lucychen-grafana merged commit 837f072 into lucychen/add_tracing Feb 20, 2025
2 checks passed
@lucychen-grafana lucychen-grafana deleted the agnestoulet/update-tracing-for-plugin branch February 20, 2025 17:56
lucychen-grafana added a commit that referenced this pull request Mar 6, 2025
@lucychen-grafana @evictorero @AgnesToulet
Image Render: Support Tracing (#586)
b1bdf72 
* Add tracing

* changed url ingestion and tried to extract traceparent

* removing unused code

* remove unused packages and add logs

* add tracing info to logs and tracing configuration

* fix config override

* fix argv.config

* fix trace initialization and starting

* remove custom log format

* update json

* change url to env

* Apply suggestions from code review

Co-authored-by: Agnès Toulet <35176601+AgnesToulet@users.noreply.github.com>

* fix yarn file

* prettier lint

* add custom trace for browser

* propagate traces back to Grafana

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

* 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

* add to readme

* update readme

* update readme

* prettier

* update tracing

* change config and docs

* Update src/plugin/v2/grpc_plugin.ts

Co-authored-by: Agnès Toulet <35176601+AgnesToulet@users.noreply.github.com>

* create new docker-compose for tracing

* add serviceName to config

* update yarn

* Apply suggestions from code review

Co-authored-by: Agnès Toulet <35176601+AgnesToulet@users.noreply.github.com>

* remove env servicename

---------

Co-authored-by: Ezequiel Victorero <ezequiel.victorero@grafana.com>
Co-authored-by: AgnesToulet <35176601+AgnesToulet@users.noreply.github.com>
lucychen-grafana added a commit that referenced this pull request Mar 12, 2025
@lucychen-grafana @evictorero @AgnesToulet
Image Render: Support Tracing (#586)
965c045 
* Add tracing

* changed url ingestion and tried to extract traceparent

* removing unused code

* remove unused packages and add logs

* add tracing info to logs and tracing configuration

* fix config override

* fix argv.config

* fix trace initialization and starting

* remove custom log format

* update json

* change url to env

* Apply suggestions from code review

Co-authored-by: Agnès Toulet <35176601+AgnesToulet@users.noreply.github.com>

* fix yarn file

* prettier lint

* add custom trace for browser

* propagate traces back to Grafana

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

* 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

* add to readme

* update readme

* update readme

* prettier

* update tracing

* change config and docs

* Update src/plugin/v2/grpc_plugin.ts

Co-authored-by: Agnès Toulet <35176601+AgnesToulet@users.noreply.github.com>

* create new docker-compose for tracing

* add serviceName to config

* update yarn

* Apply suggestions from code review

Co-authored-by: Agnès Toulet <35176601+AgnesToulet@users.noreply.github.com>

* remove env servicename

---------

Co-authored-by: Ezequiel Victorero <ezequiel.victorero@grafana.com>
Co-authored-by: AgnesToulet <35176601+AgnesToulet@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lucychen-grafana lucychen-grafana lucychen-grafana left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AgnesToulet @lucychen-grafana