Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
The Drupal 6 version of rules has the ability to create a watchdog entry, this functionality is not present in the D7 release.
Comment | File | Size | Author |
---|---|---|---|
#36 | 1349882-36-rules-watchdog-action.patch | 2.52 KB | TR |
| |||
#35 | 1349882-34-rules-watchdog-action.patch | 2.52 KB | TR |
| |||
#9 | 0001-added-description-properties-1349882-9.patch | 2.38 KB | pluess |
#6 | 1349882-6-action_create_watchdog_entry.patch | 2.02 KB | mitchell |
#4 | add-watchdog-action-1349882-4.patch | 1.76 KB | wjaspers |
Comments
Comment #1
willvincent CreditAttribution: willvincent commentedHere's a patch that adds this functionality to the available rules system actions.
Comment #2
Rory CreditAttribution: Rory commentedThis was useful, thanks.
Severity and link should probably both be optional, i.e.
And be NULL arguments by default:
Comment #3
mitchell CreditAttribution: mitchell commentedPer #2.
Comment #4
wjaspers CreditAttribution: wjaspers commentedActually, the severity is required. It defaults to the notice severity, WATCHDOG_NOTICE.
For reference: http://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/watch...
Here's an updated patch.
EDIT: Updated comment to re-issue test request.
Comment #5
Rory CreditAttribution: Rory commentedIt is not required on the form - hence the addition of the 'optional' attribute.
Comment #6
mitchell CreditAttribution: mitchell commented#4 works.
It would be better to use description fields for the text currently in the labels. I tried to edit #4 directly to fix that, but the resulting patch I'm uploading is malformed. Please update #4 soon so we can get this committed.
Comment #7
pluess CreditAttribution: pluess commentedgit apply returns
fatal: corrupt patch at line 63
for the patch of #6
Comment #8
mitchell CreditAttribution: mitchell commented^ "the resulting patch I'm uploading is malformed"
My notes and patch in #6 go with the needs review > needs work change. If you'd like to help roll a patch, I'd appreciate the help.
Use #4 if you need this now.
Comment #9
pluess CreditAttribution: pluess commentedSorry, I didn't read your notes carfully. Please find a new patch attached including your changes.
And thanks for the new action. It does what it's supposed to do: Add watchdog entries.
Comment #10
Xen CreditAttribution: Xen commentedI've just tested this patch, and it does what it says on the tin...
Comment #11
mitchell CreditAttribution: mitchell commented#9, #10: Awesome.
Comment #12
klausiThis action should use only one parameter with the data type "log_entry". Same as the watchdog event uses.
Comment #13
mitchell CreditAttribution: mitchell commentedI think @klausi is saying that the action here should reuse
_rules_system_watchdog_log_entry_info()
. @previous-patch-submitters, please provide a new patch based on this provided data type?Comment #14
mitchell CreditAttribution: mitchell commentedI started to work on this based on #12, but then realized there were more advantages to solving this in Entity API. Therefore, I propose we resolve this in #1714486: Add wrappers for watchdog's log entries. (Changing status to follow up there.)
@fago, I believe in #682722: What other modules can be candiadte for entity API, you said that this approach was "questionable" but not undesirable, and I think I've described some reasonable benefits.
I'm still progressing with php, so this patch was giving me some trouble, but I'd be happy to try again. If anyone with more experienced has the interest in getting this done, I would appreciate your efforts greatly.
Comment #15
sonicthoughts CreditAttribution: sonicthoughts commented+1
Comment #16
Angry Dan CreditAttribution: Angry Dan commentedSeems to me like the bigger issue of using entity API to wrapper the watchdog isn't getting solved any time soon. I'd say that we should commit the patch in #9 now.
Comment #17
Angry Dan CreditAttribution: Angry Dan commented... But not with "major" priority
Comment #18
PatchRanger CreditAttribution: PatchRanger commentedI'm using it and it works for me too.
As Angry Dan I'm in doubt about getting entity-wrapped log entry soon - so I support the suggestion to commit this patch.
Comment #19
mitchell CreditAttribution: mitchell commentedBetter Watchdog UI (formerly Watchdog Entity) would only need small changes to complete the entity wrapping. See #2040213: Rules Watchdog Integration.
Comment #20
fagoad #12: I think it's ok to go without the data structure as the creating the data structure first would just pose UX problems without giving any extra functionality or features.
The patch looks good (did not test it though). It misses test coverage though - we should test it in the Rules core integration test case, where all other system actions are tested. E.g. execute the action and make sure the watchdog entry has been written.
Also, what about replacements? Replacements are the only way to create proper translatable watchdog entries so we should support providing them. E.g. just add two list parameter for replacement keys and values? The nicer variant would be probably making a message module - watchdog bridge but that's another story.
Comment #21
mrded CreditAttribution: mrded commented9: 0001-added-description-properties-1349882-9.patch queued for re-testing.
Comment #22
AnybodyThe patch works good for me! It would be cool to see this feature in the next release :)
Comment #23
sonicthoughts CreditAttribution: sonicthoughts commented+1 for inclusion in next release.
Comment #24
Summit CreditAttribution: Summit commented+1 for inclusion in 2.8! greetings, Martijn
Comment #25
Summit CreditAttribution: Summit commentedComment #26
hanoiiFor what it worth, better_watchdog_ui module has a watchdog rules action.
EDIT: Oops, it was mentioned above, sorry.
Comment #27
fagosee #20.
Comment #28
MustangGB CreditAttribution: MustangGB commentedComment #29
sano CreditAttribution: sano commentedthe patch #9 did not work for me. After installing it I see a new action "Log a system message", but the action config screen contains only the Save button.
The patch applied without problems, but I wonder if the new code could be colliding with other patches I applied to the Rules module in the past. If this is the case, this just points to the troubles we expose ourselves, if patches do not get committed (fairly quickly) - folks just develop/test new patches against different code bases.
Comment #30
TR CreditAttribution: TR commented@sano: The maintainers of Rules said it wasn't ready to commit, and listed the things that still needed to be done (see e.g. #20). No one in the community worked on those things, so the patch for this new feature never got finished.
It's NOT the case in this instance that a completed patch has been sitting around ready to commit - this is an incomplete feature request with known things missing from it. In #29 you yourself found that the patch doen't work, so I don't see why you think it would have been a good idea to commit this long ago.
If you want to see this feature implemented and committed, step 1 is to update the patch so that it applies to the current version of Rules, fix the problems that you've identified, then most importantly address the issues raised by the maintainers. When we have a patch that does all this, THEN it can be reviewed and committed. That hasn't happened yet.
Comment #31
sano CreditAttribution: sano commented@TR I understand and agree with all you are writing. When I wrote what I wrote I had two issues on my mind:
1. The other Rules patches I have installed work "on my system", but the developers and potential testers of this patch might not be using them, or have installed various subsets of that collection. Therefore different people might get different testing results. Committing the "other" patches and therefore creating a common code base to test this patch against would make the situation manageable. So it is not necessarily this patch that should have been committed, but the other ones. Of course all of this might be a red herring and the problem with this patch might be unrelated to my concern.
2. When I read the #20 requirement, the last section caught my attention: "...Also, what about replacements? Replacements are the only way to create proper translatable watchdog entries so we should support providing them. E.g. just add two list parameter for replacement keys and values? The nicer variant would be probably making a message module - watchdog bridge but that's another story...."
I agree that this would be a nice to have feature, but not critical for the original requirement stated in the opening of this thread. The condition of its implementation before the patch can be committed IMHO makes that event more unlikely. I feel without that requirement the number of people willing to work on this issue might be higher.
I would like my comment not to be viewed as a critique - it is more of a lament about the general state of the FOSS development, caused by IMHO a bad incentive structure for the right people* to devote attention to - for example - completing patches like this one. I think we need to start rewarding developers for their work economically and with a better reputation reward... but that is theme for a different discussion thread and forum**.
* I am really not very good php programmer, which explains the lack of a patch attached to my comments here.
** which one? I have a proposal ready, for addressing this issue.
Comment #32
TR CreditAttribution: TR commentedComment #33
TR CreditAttribution: TR commented#2170013: Provide system action to write a db watchdog message has a better patch that what has been posted here so far, but we still need to address @fago's comments in #20.
Comment #35
TR CreditAttribution: TR commentedHere's the patch from #2170013-2: Provide system action to write a db watchdog message, renamed and rerolled to apply against HEAD. Credit goes to jonathan1055 for this patch.
Comment #36
TR CreditAttribution: TR commentedOnce more, with the 1 new coding standards problem fixed.
Comment #37
web226 CreditAttribution: web226 commentedThanks @TR your patch at #36 worked perfectly for me with Drupal 7.65 and PHP 7.2 and Rules 7.x-2.12
Comment #38
bmango CreditAttribution: bmango commentedThe patch in #36 worked great. Many thanks!
Comment #40
TR CreditAttribution: TR commentedCommitted. Needs to be ported to D8 now. The D8 action should have tests!
Comment #41
TR CreditAttribution: TR commentedShould be 8.x-3.x now ...
Comment #42
gregori.goossens CreditAttribution: gregori.goossens commentedHi,
patch #36 work for me, but i use it for write messages generated with parameters in watchdog log and due to t() function on "message" log entry Drupal create a translatable string for each different message.
Maybe we must found a solution for send variables array from Rules to watchdog callback to prevent multiple record in translation table.
What do you think about it ?
Thanks
Comment #43
TR CreditAttribution: TR commented@gregori.goossens:
Where in the code is this t() function? The patch above doesn't wrap the $message in t().
I can fix this problem if I know where the wrong code is ...
If you change this code in the drupal_watchdog action in modules/system.rules.inc:
to use
'translatable' => FALSE,
does that fix the problem?Comment #44
gregori.goossens CreditAttribution: gregori.goossens commentedThe t() function is later when message is displayed.
Maybe we can use a generic text (that could be translated later) in $message with an additional variables array.
ex :
$message = "My generic message in report : %param1 at %param2"
$variables = ["custom message 1","2020-01-30"];
with something like :
and $variables define in rules action info
Comment #45
gagarine CreditAttribution: gagarine as a volunteer commentedI don't understand the state of this feature in D8.
What is #2762157: [meta] Log to the logging service?
The action "Create a watchdog entry" is implemented for D8? It doesn't look like it (I don't see the feature in the admin).