cloudprober icon indicating copy to clipboard operation
cloudprober copied to clipboard

[Feature Request] Add support for creating metrics from HTTP probe output

Open manugarg opened this issue 5 years ago • 8 comments

Cloudprober supports building additional metrics (other than the default ones) from external probe output. We could possible do the same for HTTP probe.

manugarg avatar Oct 24 '19 16:10 manugarg

Mind if I take this one? I just learned Go and I'm eager to contribute.

mfboulos avatar Nov 09 '19 10:11 mfboulos

@mfboulos That's excellent.

Logic for this should look very similar to how we do it for external probes: https://github.com/google/cloudprober/blob/master/probes/external/payload_metrics.go

In fact, we should try to refactor the config and code in such a way that we can use them for both -- HTTP and external probe. I have a pending change to move parseValue from payload_metrics.go to metrics package, so it will be better to wait for that change (by Tue). I'll also think about sharing config and code in the meantime.

manugarg avatar Nov 10 '19 03:11 manugarg

That makes sense, configuration and metrics should be decoupled from the probe types. I'll take the time to familiarize myself with the workflow and such.

mfboulos avatar Nov 11 '19 00:11 mfboulos

I think external and payload parsing logic are sufficiently decoupled now. External probe initializes a payload parser at the time initialization: https://github.com/google/cloudprober/blob/eaaf78581bf97c71087e415da6e3a504267fb088/probes/external/external.go#L156

It updates payload metrics per target using this parser: https://github.com/google/cloudprober/blob/eaaf78581bf97c71087e415da6e3a504267fb088/probes/external/external.go#L396

I think we'd want to do something similar for HTTP probes.

manugarg avatar Nov 13 '19 19:11 manugarg

Sorry it took me so long to get around to this, I've been working on a personal project.

Anyways, I'm having a bit of trouble with a couple things. Considering I've never used cloudprober at all, it's a bit difficult for me to grasp what's going on throughout, so bear with me.

It feels like the payload metric parsing is still somewhat tightly coupled with the external probe, since PayloadMetrics takes the same format as expected from the payload in an external probe result. This seems to differ from the http probe in a way that I would have to write another method in the payload parser and modify the http proto to accommodate for that.

If I'm missing something that indicates that they're indeed not as tightly coupled as I'm seeing, then it may be due to the fact that I'm just not entirely sure what metrics we're passing from the http probe results to begin with.

Any further clarity on those fronts would help me immensely in resolving this issue.

mfboulos avatar Nov 23 '19 02:11 mfboulos

payloadParser.PayloadMetrics expects payload to just be a text that looks like this:

For external probe it's produced by either running an external command: https://github.com/google/cloudprober/blob/5d8563a26e5c80fd65dd426c9901fb3b99708e76/probes/external/external.go#L522,

or by sending a probe request to probe server: https://github.com/google/cloudprober/blob/5d8563a26e5c80fd65dd426c9901fb3b99708e76/probes/external/external.go#L454

In case of HTTP probe, payload will be HTTP response body in the similar format that I described above. Currently we are not parsing HTTP response body (except for the option -- export_response_as_metrics).

(Just FYI: I am going to be away from work for next 6 days. I'll read your response after that.)

manugarg avatar Nov 24 '19 08:11 manugarg

Alright, that makes sense! I've added a couple optional fields to the config proto to mirror the changes on the external probe, which should allow configuration with the same level of freedom.

However, I'm having a bit of trouble with protoc that I can't resolve. With the same metrics payload proto import as the external probe config proto, protoc doesn't want to resolve the import, regardless of whether or not I set the proto path to a directory that can reach both protos.

This is pretty much the last blocker before I fully hash this out.

mfboulos avatar Nov 24 '19 10:11 mfboulos

There is a script in tools directory to generate protobuf code, did you give that a try?

manugarg avatar Nov 24 '19 22:11 manugarg