socketry / cloudflare

An asynchronous Ruby wrapper for the CloudFlare V4 API.
138 stars 86 forks source link

Add logs to a cloudflare zone #38

Open simongareste opened 5 years ago

simongareste commented 5 years ago

Hi,

I needed to add the logs functionality, so that we can get them from CloudFlare and store them in our premises.

I did this commit, which has a (important) caveat grearding the result. The cloudflare doc states that they do not, for the logs, send back one JSON, but many lines of JSON, so basically something of the like:

{"ClientIP":"1.2.3.4","ClientRequestHost":"fqdn.domain.name","ClientRequestMethod":"GET","ClientRequestURI":"/file1.png"}
{"ClientIP":"2.3.4.5","ClientRequestHost":"fqdn.domain.name","ClientRequestMethod":"GET","ClientRequestURI":"/file1.png"}
{"ClientIP":"4.5.6.7","ClientRequestHost":"fqdn.domain.name","ClientRequestMethod":"GET","ClientRequestURI":"/other_file.png"}

As you can see, each line is a valid JSON, but the result itself is not. So what I did was adding options so that we can force text/plain instead of application/json, and in those case stop the parsing and split it instead. The JSON parsing is then done by the Logs class, as it should. But if we get an error from CloudFlare, I do not know how I can figure it out. Given that in the connection.rb file, you set (in the initializer)

            # Convert HTTP API responses to our own internal response class:
            super(url, headers: headers, accept: 'application/json', **options) do |response|
                Response.new(response.request.url, response.body, **options)
            end

Because of this, the response.code (which would help) is not accessible. But changing the Response initializer signature is IMO an important change. I'm uncertain on which way would be best to continue, so your input is very welcom.

ioquatix commented 5 years ago

There is a good discussion here about the right content type to use: https://lesspress.net/blog/newline-delimited-json/

ioquatix commented 5 years ago

It might make sense to consider using content-type: application/json; boundary=NL

simongareste commented 5 years ago

Thanks! Never heard of that, I'll take a look and try to set it up.

ioquatix commented 5 years ago

Yeah it’s pretty uncommon but I think it might be the right solution.

ioquatix commented 5 years ago

I've been reworking the code base on top of async-rest. Do you mind reworking this PR onto of the current master branch?

ioquatix commented 5 years ago

Okay so I tried to implement this but then realised I have no way to test it because it's an enterprise feature and I don't have access to that. I've emailed Cloudflare support and asked them to give me some kind of access.