Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mix types for the context argument #12

Open
erickhun opened this issue Dec 12, 2019 · 1 comment
Open

Mix types for the context argument #12

erickhun opened this issue Dec 12, 2019 · 1 comment

Comments

@erickhun
Copy link
Collaborator

erickhun commented Dec 12, 2019

Hi @esclapes @colinscape

Wanted to get your thoughts on making BuffLog more flexible and make it "safer" to use.

There was an incident that happen when I've integrated BuffLog with the analyze workers

How it works now is that we call Monolog directly but if the 2nd param type isn't correct, it will throw an exception and crash the script.

I've also noticed some challenges when replacing all the var_dump in the analyze code base by trying to guess what type are the variables.

Here my question/suggestion:

I like the flexibility of a var_dump($param) because we can use it without worrying much about about what $param is. I also believe it's used because it will never crash the code.

  • What do you think on allowing any type of arguments: array, int, float, string for the 2nd argument? It seems like in the PHP world it's called a "mixed" type if we can't predict the type of a parameter.
  • Also we can check if the 1st parameter is a string and not throw any exception if not.

So we would have prototype that looks lilke that:

BuffLog::notice(string , mixed); 
BuffLog::debug(string , mixed); 
...

// instead of
BuffLog::notice(string , array); 
BuffLog::debug(string , array); 

Thoughts? :)

@colinscape
Copy link

I definitely like the idea of making the logger as safe to use as possible. We definitely don't want processes crashing because of a failed logging call.

I guess the question is then whether we accept the multitude of possible types that could be sent, or whether we reject them, and log the error.

I'm tending towards the latter as a means of helping to make it clear what the logging method needs. Passing numbers or strings as the second parameter can probably be achieved in much the same way by incorporating them into the message, and providing a 'raw' value like "3" doesn't give any information on what that value is (is it the number of profile, or the number of updates or what?) and so having something appear in Datadog like {"value": 3} won't provide a lot of use.

We'd just want to make sure that if we log mis-use of the log method that it doesn't get into an infinite loop!! 😅

Would love to hear other perspectives and thoughts round this though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants