vectordotdev
fix(sinks/gcp/pubsub.rs): remove pubsub log only restriction
Looks like the pubsub sink was restricted to logs only as part of a wide change, but does not need to be. This simply removes that restiction.
Deploy Preview for vector-project canceled.
| Name | Link |
|---|---|
| Latest commit | 2f3cf337d2aead383656d5077fcc046fc6bc189c |
| Latest deploy log | https://app.netlify.com/sites/vector-project/deploys/64c1d63b6b67e5000833bcfa |
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
Deploy Preview for vrl-playground canceled.
| Name | Link |
|---|---|
| Latest commit | 2f3cf337d2aead383656d5077fcc046fc6bc189c |
| Latest deploy log | https://app.netlify.com/sites/vrl-playground/deploys/64c1d63bb631d000076a77b3 |
@jszwedko I can certainly try, need to get familiar w/ the code base/rust in general :-) if you've got any pointers/examples of similar tests I'll absolutely take them.
@jszwedko I can certainly try, need to get familiar w/ the code base/rust in general :-) if you've got any pointers/examples of similar tests I'll absolutely take them.
Sure thing! You can find the existing tests under https://github.com/vectordotdev/vector/blob/92e320a75f8774956af6370947679df6f2ceda1e/src/sinks/gcp/pubsub.rs#L269-L444 . I think you could copy/paste the publish_events one and modify it to use metrics instead of logs.
Thanks, looking into it. Is there a standard method in tests to generate random metrics, like ranadom_events_with_stream but metrics? Looking at other sinks I'm mostly finding hardcoded string representations/etc.
@jszwedko looking at other sinks, like the influxdb one (specifically the metrics component) the test is essentially the same (using the random events stream), which the pubsub sink already uses, so I'd think thats "good enough" since those events are not specifically logs nor metrics. also local testing I can ingest remote_write metrics (source) -> pubsub (as metrics) as well as pubsub (source) as metrics -> console out.
Thanks for taking a look @sjoeboo !
I think you are right that there isn't an equivalent function to generate random metrics. The InfluxDB metrics sinks seem to have helper functions that do this. See:
https://github.com/vectordotdev/vector/blob/421b421bb988335316417c80129014ff80179246/src/sinks/influxdb/metrics.rs#L1195-L1209
Which is then used by the test:
https://github.com/vectordotdev/vector/blob/421b421bb988335316417c80129014ff80179246/src/sinks/influxdb/metrics.rs#L1019
It looks like random_events_with_stream only generates logs:
https://github.com/vectordotdev/vector/blob/421b421bb988335316417c80129014ff80179246/src/test_util/mod.rs#L277-L290
We could add another helper there that generates a random stream of metrics though (and maybe rename that one to random_logs_with_stream.
Sorry, I know this is probably a bit more work than you were expecting to do. It's good to know it worked locally when you tested it. I think the value of the tests, then, would be to make sure that functionality doesn't break accidentally.
Let me know if that makes sense!
@jszwedko @StephenWakely What is the status of pull request? Looks like it got it abandoned a bit :( . I will be happy to contribute to make it go through. Should i fork it and add the suggested changes?
@jszwedko @StephenWakely What is the status of pull request? Looks like it got it abandoned a bit :( . I will be happy to contribute to make it go through. Should i fork it and add the suggested changes?
@genadipost Hey! Yes, it looks like the contributor fell off of this one. If you'd like you can create a new PR based on this branch that incorporates the feedback.
