Port api logging into synapse-base about synapse-base

[spruce-bruce]

❄️ 2️⃣ ⬇️

[zpchavez]

⭕

[paulstatezny]

I think the AC's could use a little more specific language to make the task more clear. Also, adding the Tasks section would be good to this end.

On a different note, perhaps this should be more generic than just "restricted access logging". Should service providers be able to register endpoints for other types of logging? (Logging other events.)

Finally -- I think the best way to do this might be to extend some Silex or Symfony functionality in order to make this really elegant. It would be cool if we could do something like:

$app->match('/some/path', 'some.controller:rest')
    ->method('GET|PUT|POST')
    ->bind('some-entity')
    ->log(SomeClassWithLogTypeConstants::RESTRICTED_ACCESS, 'namespace.in.syslog');

[spruce-bruce]

@paulstatezny I agree about your whole Silex/Symfony functionality thing. That's what I was getting at with 'Consider extending the $app->match() function to register endpoints with the logger'

I left it a little vague because I don't know what this is going to look like - it needs some planning/discussion about the best way to port this in. I added points so it doesn't show up as a problem in the overview, but maybe there's another way to prevent this incomplete issue from showing up as a problem in the overview?

[paulstatezny]

That all sounds good, thanks!

[spruce-bruce]

I think the last thing to consider is how best to pair logged events with namespaces. Is it safe to assume that POST should use entity.create namespace or will it be common to want to use some other namespace in a POST method?

I can imagine a service that just looks at the request and looks up the namespace based on the registered route/namespace pair and says "ah we have a POST to /endpoint therefore this is measure.changes.endpoint.create", but I also like the idea of leaving a little in the hands of the controller when it's logging specific events.

Like when a controller needs to log a create.fail for example, is it enough for us to call $logger->logFail($request, $response) and have the logging service determine the namespace from the context of the Request object? I like this idea but I'm worried it may be too rigid. Any thoughts?

[baohx2000]

Didn't see it, but to differentiate it from simply being able to parse apache logs using uri & HTTP method, I'm thinking this logging should always include the user ID if one exists as part of the log metadata (if one exists). Also, instead of adding yet another identifier, why not use the same identifier used in bind()?

[baohx2000]

Perhaps naming used in bind() should be changed to be more in line with what's wanted for the logging identifier?

[spruce-bruce]

@baohx2000 agreed on both points. The logging should try to log the id of the actor (current user) and the id of the entity being acted upon when either are available.

bind() sounds like a reasonable string to use as part of the namespace string.

ex:
measure.changes.BINDVALUE.{new|edit|remove} the last bit of the namespace can be set based on if the request is POST (new), PUT (edit) or DELETE (remove). maybe we just put post, put or delete right in the namespace instead of new/edit/remove.

[baohx2000]

Agree that we should just use HTTP method names and not mutate them. Really, PUT should be "replace". PATCH would be edit. There are also inevitably APIs which simply initiate some sort of functionality, so it's not like it's creating a new thing.

[spruce-bruce]

To me that's an argument for not using the HTTP methods in the namespace. Ideally these identifiers should be a shorthand for what action is taking place without having to have special knowledge of the application.

IE, I know when I do a POST to /some-endpoint that it doesn't actually perform an operation on an entity, but I would hope that I don't need this special knowledge to interpret the logs.

I think it is reasonable to include the HTTP method in the log regardless, but maybe not as part of the identifier if we think that the HTTP method itself can't be used to derive the actual CRUD action that's taking place.

There's another question - do we want to log from the perspective of the endpoint or from the perspective of the entity? What I've been thinking is that we're logging data access, basically just CRUD on our entities, but maybe we need to think about endpoint logging as a separate idea with special cases?

The example i have in mind is this:
We have two user types - typeA and tybeB. They both have access to unique endpoints that access the same entity. Lets define these endpoints as GET '/typeA/entity/{id}' and GET /typeB/entity/{id}. These endpoints return different data, but it's still data about the same entity.

Logging from the perspective of the entity will log the same events for both of these endpoints, ie, the measure.access.entity event. The users involved will be logged as part of the message but not in the identifier.

This is semantics, but if we're using the bind() value we'll end up with a different log namespace for each of these endpoints. It can be architected that the only difference is the last section, but I think I'm leaning away from using bind() now and liking the idea of operations on an entity being logged with the same identifier no matter what the endpoint is, and then information about the endpoint will be logged in the message itself as context information.