transparency-dev / trillian-tessera

Go library for building tile-based transparency logs (tlogs)
Apache License 2.0
11 stars 11 forks source link

Hammer should support distinct URL for read and write #161

Closed AlCutter closed 3 months ago

AlCutter commented 3 months ago

Currently, the hammer takes a single "root" URL which it uses to construct read and write requests to the log. This works if the personality provides proxy services to read from the storage, but in the more general case the expectation is that log read clients will hit the underlying static resources directly.

Having the ability to (optionally) specify distinct URLs for read and write requests would allow the hammer to exercise this more generally applicable path.

phbnf commented 3 months ago

Would it be possible to also change the add endpoint, rather than forcing it to "add"? "add" is not a spec-specific endpoint, and is different for CT (it's actually even more different for CT since there are two endpoints, but let's not get there yet :) )

roger2hk commented 3 months ago

Would it be possible to also change the add endpoint, rather than forcing it to "add"? "add" is not a spec-specific endpoint, and is different for CT (it's actually even more different for CT since there are two endpoints, but let's not get there yet :) )

Turn the hammer into a library, so we can have personality hammer?

mhutchinson commented 3 months ago

Would it be possible to also change the add endpoint, rather than forcing it to "add"? "add" is not a spec-specific endpoint, and is different for CT (it's actually even more different for CT since there are two endpoints, but let's not get there yet :) )

This was going to be my original approach but @AlCutter convinced me otherwise with the great reason that the write handler already assumes other details about the endpoint such as POST data, encoding types, and response codes. At this point, keeping it pinned to expect /add as the name seems pretty mundane and it keeps it clear that this is a hammer for our example personalities only and does not support ad-hoc configuration.

Turn the hammer into a library, so we can have personality hammer?

That's a possibility and the hammer has somewhat been designed to allow for this, but it feels too early to take on that burden now. I think the best course of action is to take the viewpoint that this is an example conformance load-test for example personalities, and users should copy/paste/extend in much the same way that we are expecting with the example personalities. Once we have seen 2 extensions, we'll have a much better idea of what the shared logic is.

mhutchinson commented 3 months ago

Marking this as closed because it solves the problems that this issue was scoped to (allowing our example personalities to be load tested, where the write endpoint is at a different base URL than the read base URLs). Please open another issue with a clear exit criteria for other features requested of the hammer.