-
Notifications
You must be signed in to change notification settings - Fork 115
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
Add log_level spec #332
Add log_level spec #332
Conversation
💔 Build FailedExpand to view the summary
Build stats
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
@felixbarny This looks excellent to me as-is. Is there more to do before we take is out of draft and start getting individual teams to review? |
I’d usually try to get feedback from an agent that did not give feedback so far or were it might be tricky for them to implement the spec. In this case, I’d suggest asking the RUM team for a review before opening it up to a wider audience. |
This is a bit problematic for .NET. On .NET Core we integrate with The reason we integrate with
This means, everything logs on
So the
Then here is what happens:
It crashes way before we could do anything. I don't wanna push back and if needed I'm ok with digging deeper, but for .NET this would mean that
|
Now.. having said that above: of course that only happens when the values are coming from a known config file. We can still fetch the suggested defaults from central config and map those. So from my point of view, having something common for central config is possible. But then for .NET
I leave out here the combination of the two, for that, there is already an issue in the .NET repo. |
@gregkalapos Thanks for the insights. I think your second solution will be fine, as we discussed in the agents meeting today. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
We seem to have a critical mass of approvals here, so let this be a last call for comment: This spec will be merged and approved October 5, 2020. |
Supersedes #321
Note that the implementation of this spec also includes adding the option to Kibana.