Skip to content

Conversation

@th3M1ke
Copy link

@th3M1ke th3M1ke commented Mar 11, 2025

This pull request introduces several enhancements and new features to the Prometheus client and its integration with CodeMirror. The most significant changes include adding support for fetching Prometheus server flags, creating a CodeMirror Prometheus client adapter, and updating various components to utilize the new adapter.

Enhancements and New Features:

  • Support for Prometheus Server Flags:

    • Added FlagsResponse type to prometheus/src/model/api-types.ts.
    • Introduced flags method in PrometheusClient interface and implemented it in prometheus/src/model/prometheus-client.ts. [1] [2]
    • Updated prometheus/src/plugins/prometheus-datasource.tsx to use the new flags method. [1] [2]
  • CodeMirror Prometheus Client Adapter:

    • Created createCodeMirrorPromClient function in prometheus/src/model/prometheus-client.codemirror-adapter.ts.
    • Updated PrometheusTimeSeriesQueryEditor to use the new CodeMirror Prometheus client. [1] [2] [3]
    • Updated PrometheusPromQLVariableEditor to use the new CodeMirror Prometheus client. [1] [2] [3]

Resolves: #78

…or-promql package

Signed-off-by: Mykhailo Semenchenko <mike.semenchenko@gmail.com>
@th3M1ke th3M1ke force-pushed the propagate-heaaders branch from d6c4d86 to 17b054f Compare March 11, 2025 16:30
@Nexucis
Copy link
Member

Nexucis commented Mar 11, 2025

I don't see why do you need to implement the method flag. It's not used in the plugin 🤔

@th3M1ke
Copy link
Author

th3M1ke commented Mar 12, 2025

Thanks for your feedback! 🙏 The method flag isn't used in the Perses Prometheus client, but it is required for the Codemirror one. I thought it would be a good idea to keep the clients consistent and aligned in structure. However, if you prefer, we can mock it instead

@Nexucis
Copy link
Member

Nexucis commented Mar 18, 2025

Oups sorry for the time it took to come back to you and thank you for pinging me on slack.

So I would prefer to mock it for the moment. If at some points it's necessary we can finally implement it, it won't cost that much.

On another topic, I don't get how this change can fix the issue you raised (#78). Propagating the headers is not the job of the plugin but the job of the proxy on the backend side. So this fill weird this PR is fixing that.

Finally, can you

  • sign your commit following the DCO procedure you can find in the DCO job ?
  • rebase your PR to get the last changes of the CI that should unlock some issue we currently have with Cuelang
  • fix all issues raised by the linter

Thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Perses does not propagate proxy headers to all api calls

2 participants