Log Flow activity #18904
Log Flow activity #18904
Conversation
|
/backport to stable18 |
blizzz
requested review from
ChristophWurst,
juliusknorr,
nickvergessen and
rullzer
| } | ||
|
|
||
| public function setOperation(?IOperation $operation): LogContext { | ||
| if($operation instanceof IOperation) { |
There was a problem hiding this comment.
given the type hint on the parameter this change is a bit weird. is this a null check?
also, why is the parameter nullable when passing in null doesn't actually do anything? 🤔
There was a problem hiding this comment.
doesn't bloat the code in RuleMatcher to much, where it is not guaranteed that an operation is set (you can work around with using deprecated API)
There was a problem hiding this comment.
If nothing is set, log should not crash
There was a problem hiding this comment.
let me rephrase: did you mean to if ($operation !== null)? Because except for null the $operation will always be of type IOperation as per type hint.
| } | ||
|
|
||
| public function setEntity(?IEntity $entity): LogContext { | ||
| if($entity instanceof IEntity) { |
lib/public/Log/IDataLogger.php
Outdated
| * | ||
| * @since 18.0.0 | ||
| */ | ||
| public function logData(array $data, array $context = []): void; |
There was a problem hiding this comment.
should we also add something like a message parameter here? all other logger methods either take a string or an exception that contains a message.
There was a problem hiding this comment.
imho this should be just part of the data array that is being passed into
|
@ChristophWurst ok? others – reviews 🌻 ? |
|
If this has a chance of being logged to the main log the the logreader will need to be able to handle it |
|
@icewind1991 are Exceptions handled specifically or in a generic way? |
|
They are handled specifically to have them fancy formatted. For messages like this, having a guaranteed |
|
|
I would prefer to have a |
blizzz
added
2. developing
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
also ensure it plays well with current log reader Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
blizzz
force-pushed
the
enh/noid/flow-logging
branch
from
3622d3e to
e008444
Compare
|
A message parameter is not enough for the logreader, since it just tries to print the delivered payload if it does not match any Exception pattern. What works better is to split the payload into a message and data part. Then the log reader works as it is now (extra fields are not being show but can be copied rawly). I don't like the transformation kung foo n LogDetails however, but that's necessary to another field to the log output. Back to review. |
blizzz
added
3. to review
|
are we fine with this now? @icewind1991 @rullzer @ChristophWurst |
| * Interface IDataLogger | ||
| * | ||
| * @package OCP\Log | ||
| * @since 18.0.1 |
|
backport to stable18 in #19396 |
Example content:
💄 Prettified
cat data/flow.log | jq{ "reqId": "X5XP4vROpcGCoaE6Ria2", "level": 1, "time": "2020-01-15T09:32:29+00:00", "remoteAddr": "127.0.0.1", "user": "master", "app": "workflowengine", "method": "GET", "url": "/master/ocs/v2.php/apps/flow_http_requests/api/v1/f/m7b43vgb?action=ping", "message": { "operation": { "class": "OCA\\Talk\\Flow\\Operation", "name": "Write to conversation" }, "entity": { "class": "OCA\\FlowHttpRequests\\Flow\\RequestEntity", "name": "Web Request" }, "eventName": "FlowHttpRequest::incomingRequest", "description": "Flow activated by event" }, "userAgent": "curl/7.68.0", "version": "18.0.0.8" } { "reqId": "X5XP4vROpcGCoaE6Ria2", "level": 1, "time": "2020-01-15T09:32:29+00:00", "remoteAddr": "127.0.0.1", "user": "master", "app": "workflowengine", "method": "GET", "url": "/master/ocs/v2.php/apps/flow_http_requests/api/v1/f/m7b43vgb?action=ping", "message": { "scopes": [ { "scope": "admin" }, { "scope": "user", "uid": "master" } ], "entity": { "class": "OCA\\FlowHttpRequests\\Flow\\RequestEntity", "name": "Web Request" }, "operation": { "class": "OCA\\Talk\\Flow\\Operation", "name": "Write to conversation" }, "description": "Flow activation: rules were requested" }, "userAgent": "curl/7.68.0", "version": "18.0.0.8" } { "reqId": "X5XP4vROpcGCoaE6Ria2", "level": 1, "time": "2020-01-15T09:32:29+00:00", "remoteAddr": "127.0.0.1", "user": "master", "app": "workflowengine", "method": "GET", "url": "/master/ocs/v2.php/apps/flow_http_requests/api/v1/f/m7b43vgb?action=ping", "message": { "entity": { "class": "OCA\\FlowHttpRequests\\Flow\\RequestEntity", "name": "Web Request" }, "operation": { "class": "OCA\\Talk\\Flow\\Operation", "name": "Write to conversation" }, "configuration": { "id": 60, "class": "OCA\\Talk\\Flow\\Operation", "name": "", "checks": "[81]", "operation": "{\"m\":3,\"t\":\"wfwqkvsc\"}", "events": "[\"FlowHttpRequest::incomingRequest\"]", "entity": "OCA\\FlowHttpRequests\\Flow\\RequestEntity", "scope_type": 1, "scope_actor_id": "master" }, "description": "Flow rule qualified to run" }, "userAgent": "curl/7.68.0", "version": "18.0.0.8" } { "reqId": "X5XP4vROpcGCoaE6Ria2", "level": 1, "time": "2020-01-15T09:32:29+00:00", "remoteAddr": "127.0.0.1", "user": "master", "app": "workflowengine", "method": "GET", "url": "/master/ocs/v2.php/apps/flow_http_requests/api/v1/f/m7b43vgb?action=ping", "message": { "entity": { "class": "OCA\\FlowHttpRequests\\Flow\\RequestEntity", "name": "Web Request" }, "operation": { "class": "OCA\\Talk\\Flow\\Operation", "name": "Write to conversation" }, "configuration": [ { "id": 60, "class": "OCA\\Talk\\Flow\\Operation", "name": "", "checks": "[81]", "operation": "{\"m\":3,\"t\":\"wfwqkvsc\"}", "events": "[\"FlowHttpRequest::incomingRequest\"]", "entity": "OCA\\FlowHttpRequests\\Flow\\RequestEntity", "scope_type": 1, "scope_actor_id": "master" } ], "description": "All qualified flow configurations are going to run" }, "userAgent": "curl/7.68.0", "version": "18.0.0.8" } { "reqId": "X5XP4vROpcGCoaE6Ria2", "level": 1, "time": "2020-01-15T09:32:29+00:00", "remoteAddr": "127.0.0.1", "user": "master", "app": "workflowengine", "method": "GET", "url": "/master/ocs/v2.php/apps/flow_http_requests/api/v1/f/m7b43vgb?action=ping", "message": { "operation": { "class": "OCA\\Talk\\Flow\\Operation", "name": "Write to conversation" }, "entity": { "class": "OCA\\FlowHttpRequests\\Flow\\RequestEntity", "name": "Web Request" }, "eventName": "FlowHttpRequest::incomingRequest", "description": "Flow handling for event done" }, "userAgent": "curl/7.68.0", "version": "18.0.0.8" } { "reqId": "X5XP4vROpcGCoaE6Ria2", "level": 1, "time": "2020-01-15T09:32:29+00:00", "remoteAddr": "127.0.0.1", "user": "master", "app": "workflowengine", "method": "GET", "url": "/master/ocs/v2.php/apps/flow_http_requests/api/v1/f/m7b43vgb?action=ping", "message": { "operation": { "class": "OCA\\WorkflowScript\\Operation", "name": "External scripts" }, "entity": { "class": "OCA\\FlowHttpRequests\\Flow\\RequestEntity", "name": "Web Request" }, "eventName": "FlowHttpRequest::incomingRequest", "description": "Flow activated by event" }, "userAgent": "curl/7.68.0", "version": "18.0.0.8" } { "reqId": "X5XP4vROpcGCoaE6Ria2", "level": 1, "time": "2020-01-15T09:32:29+00:00", "remoteAddr": "127.0.0.1", "user": "master", "app": "workflowengine", "method": "GET", "url": "/master/ocs/v2.php/apps/flow_http_requests/api/v1/f/m7b43vgb?action=ping", "message": { "operation": { "class": "OCA\\WorkflowScript\\Operation", "name": "External scripts" }, "entity": { "class": "OCA\\FlowHttpRequests\\Flow\\RequestEntity", "name": "Web Request" }, "eventName": "FlowHttpRequest::incomingRequest", "description": "Flow handling for event done" }, "userAgent": "curl/7.68.0", "version": "18.0.0.8" }