{"action":"created","issue":{"url":"https://api.github.com/repos/elastic/logstash/issues/5121","repository_url":"https://api.github.com/repos/elastic/logstash","labels_url":"https://api.github.com/repos/elastic/logstash/issues/5121/labels{/name}","comments_url":"https://api.github.com/repos/elastic/logstash/issues/5121/comments","events_url":"https://api.github.com/repos/elastic/logstash/issues/5121/events","html_url":"https://github.com/elastic/logstash/issues/5121","id":148461197,"number":5121,"title":"The Principle of Least Astonishment and the New Event API","user":{"login":"andrewvc","id":131427,"avatar_url":"https://avatars.githubusercontent.com/u/131427?v=3","gravatar_id":"","url":"https://api.github.com/users/andrewvc","html_url":"https://github.com/andrewvc","followers_url":"https://api.github.com/users/andrewvc/followers","following_url":"https://api.github.com/users/andrewvc/following{/other_user}","gists_url":"https://api.github.com/users/andrewvc/gists{/gist_id}","starred_url":"https://api.github.com/users/andrewvc/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/andrewvc/subscriptions","organizations_url":"https://api.github.com/users/andrewvc/orgs","repos_url":"https://api.github.com/users/andrewvc/repos","events_url":"https://api.github.com/users/andrewvc/events{/privacy}","received_events_url":"https://api.github.com/users/andrewvc/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/elastic/logstash/labels/design","name":"design","color":"eb6420"},{"url":"https://api.github.com/repos/elastic/logstash/labels/discuss","name":"discuss","color":"cc317c"},{"url":"https://api.github.com/repos/elastic/logstash/labels/v5.0.0","name":"v5.0.0","color":"ededed"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":13,"created_at":"2016-04-14T19:37:58Z","updated_at":"2016-04-15T01:18:36Z","closed_at":null,"body":"I've given a lot of thought to some past discussions WRT the new Event API and I think there's a solution below that will make everyone happy.\r\n\r\nWe have a bad API/user experience problem in Logstash 5.0. `event[‘foo’][‘bar’] = ‘blah’` and `event['foo'].downcase!` should either set the value (as it currently does) or raise an error if it doesn't. This follows the [principle of least astonishment](https://en.wikipedia.org/wiki/Principle_of_least_astonishment), a key tool in UX design that definitely applies here.  It doesn't matter which one it does, but it should perform one of those two actions. Setting the value is the obvious thing it would do. If it doesn't do the obvious thing it should raise an error. \r\n\r\nAn API where things can easily look like they do something, and in fact used to be the recommended way of doing things (going by our own code), but now does nothing with no feedback is bad. \r\n\r\nSilent failure is the worst possible experience for a user. We should not make that choice intentionally. Even if they eventually learn and understand this API it doesn't mean they would (or should) like it.\r\n\r\nRegardless of how well we document it, or how many people can be trained to not do it, for a developer new or old that's still a bad experience. People could easily make an innocent mistake.\r\n\r\n**What's the fix?**\r\n\r\nPreserving the old behavior is a controversial topic. Doing so would require a Java event that could operate equally well on Ruby objects as well as Java ones and be switched between ruby and Java internal representations depending on whether it was being used in a Java plugin.\r\n\r\nI believe we can all agree as a lowest common denominator that the following would be better than what's currently in 5.0.0. We simply should invoke `.freeze` recursively on any ruby objects returned. This would only be a handful of lines of code, and would clearly warn and educate users to not mutate data.\r\n\r\nAll this means is that:\r\n\r\n`event['foo'].downcase! # doesn't work with current java event, but fails silently`\r\n\r\nwould raise an exception forcing the user to rewrite it correctly as\r\n\r\n`event['foo'] = event['foo'].downcase # does the right thing`\r\n\r\nWe could even add extra info to FrozenString errors letting users know about event mutability.\r\n\r\nThank you all for considering this proposal! I think it may still be worth talking about a 'full' fix where nothing would change but I'm not so certain that perspective is very popular ATM, though I think it is technically achievable."},"comment":{"url":"https://api.github.com/repos/elastic/logstash/issues/comments/210235214","html_url":"https://github.com/elastic/logstash/issues/5121#issuecomment-210235214","issue_url":"https://api.github.com/repos/elastic/logstash/issues/5121","id":210235214,"user":{"login":"colinsurprenant","id":2010,"avatar_url":"https://avatars.githubusercontent.com/u/2010?v=3","gravatar_id":"","url":"https://api.github.com/users/colinsurprenant","html_url":"https://github.com/colinsurprenant","followers_url":"https://api.github.com/users/colinsurprenant/followers","following_url":"https://api.github.com/users/colinsurprenant/following{/other_user}","gists_url":"https://api.github.com/users/colinsurprenant/gists{/gist_id}","starred_url":"https://api.github.com/users/colinsurprenant/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/colinsurprenant/subscriptions","organizations_url":"https://api.github.com/users/colinsurprenant/orgs","repos_url":"https://api.github.com/users/colinsurprenant/repos","events_url":"https://api.github.com/users/colinsurprenant/events{/privacy}","received_events_url":"https://api.github.com/users/colinsurprenant/received_events","type":"User","site_admin":false},"created_at":"2016-04-15T01:18:36Z","updated_at":"2016-04-15T01:18:36Z","body":"@andrewvc again, breaking compatibility in a major version release is OK. "}}