Skip to content

Conversation

@bwplotka
Copy link
Collaborator

@bwplotka bwplotka commented Aug 18, 2025

Relates to b/425614387

This adds prw2gcm binary to our Prometheus fork image that forwards PRW 2.x requests as GCM v3 gRPC calls.

NOTE: nhcb and potential classic histogram support will be added in the next PRs.

Rationales

While initially we dismissed writing this proxy, given the potential OpenTelemetry Collector's PRW2.0 -> Otel data model -> GCM/OTLP flow recent (experimental) development... I decided to write it anyway 🙈 I put some limited time for it, but I wanted to answer some questions:

Turns our the conversion is straightforward for Prometheus use (single sample) and a little bit more involved with multiple samples, but still... manageable to write (with some e2e tests!) in ~8 SWE-h 🎉 Also spent some extra time trying to figure out how to pass freaking auth token from the HTTP header into gRPC call 🙈

Apart from learnings, I do think it's useful to maintain and even ship with our Prometheus fork image, because:

  • Our GCM ingestion can be inspired (or even straight import) this code.
  • GCM PRW support is incoming, but will take time. With this prw2gcm binary we can:
    • Start heavy testing of vanilla Prometheus -> GCM, without significant complexities of the untested Otel path. I plan to use this binary in our e2e GCM tests in the next PRs

EDIT: See #255 for tests using prw2gcm.

  • Users will be able to use vanilla Prometheus with prw2gcm sidecar from day 1, allowing us to unfork practically sooner. Again, without Otel 3 model conversions complexity and overhead.

@bwplotka bwplotka force-pushed the prw-test branch 3 times, most recently from 1bab242 to 8ab3894 Compare August 20, 2025 16:32
@bwplotka bwplotka changed the title feat: Add prw2gcm binary; e2e test vanilla Prometheus + GCM via prw2gcm feat: Add prw2gcm binary Aug 20, 2025
@bwplotka bwplotka marked this pull request as ready for review August 20, 2025 16:36
@bwplotka bwplotka force-pushed the prw-test branch 3 times, most recently from 118cdb1 to 76002a9 Compare August 20, 2025 19:43
@pintohutch
Copy link
Collaborator

Thanks Bartek - what's your rationale specifically for not creating a dedicated prw2gcm image?

initialSeriesTsIndex = append(initialSeriesTsIndex, i)
}

// Go through samples. We have to do it over sample dimension, not series, given the
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we just iterate over the series dimension and send each point in it's own CreateTimeSeries request?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Would it be efficient enough for everyone to have batchLen = 1 always?

@bwplotka
Copy link
Collaborator Author

bwplotka commented Aug 28, 2025

Thanks Bartek - what's your rationale specifically for not creating a dedicated prw2gcm image?

Just simplicity, similarly we don't have separate image for promtool. It is meant to be a tmp thing next to Prometheus, so feels like a good enough choice.

We can make it separate, no problem. Should we? If yes, should it be in prometheus-engine that already builds multiple images or here?

I like having it here, because:

  • We can test directly in our e2e gcm test suite which is maintained (and run on CI here). In fact we test exact version of the proxy binary since the code for it is here too.
  • This proxy depends on Prometheus AND export package, so putting it to operator module that has more deps feels unnecessary.

feat: switch to dimension based approach for bufferes samples

Signed-off-by: bwplotka <bwplotka@gmail.com>
@pintohutch
Copy link
Collaborator

Those reasons seem compelling enough to me to keep it in this image :)

@bwplotka
Copy link
Collaborator Author

Let's merge the initial state - we won't be mentioning this tool anywhere until it's ready.

Thanks!

@bwplotka bwplotka merged commit f7f4d37 into release-2.53.5-gmp Aug 29, 2025
5 checks passed
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.

3 participants