Init Logger before Configs #823
Conversation
omg you're right. how silly. this will need fixing though, see the red X to see build errors. |
metrictank.go
Outdated
| @@ -175,6 +176,9 @@ func main() { | |||
| case 6: | |||
| log.Level(log.FATAL) | |||
| } | |||
| mdata.LogLevel = logLevel | |||
| inKafkaMdm.LogLevel = logLevel | |||
| api.LogLevel = logLevel | |||
There was a problem hiding this comment.
why did you move the assignments down instead of just keeping them as is?
There was a problem hiding this comment.
Honestly, a testing artifact. I did think that since we're initializing the logger first, we'd want to configure it first.
|
T'was just gofmt. Hmm, I must have messed up following our workflow since it's our master branch. If you could squash the commits when merging, that'd be easiest :) |
metrictank.go
Outdated
| /*********************************** | ||
| Initialize Logger | ||
| ***********************************/ | ||
| log.NewLogger(0, "console", fmt.Sprintf(`{"level": %d, "formatting":false}`, logLevel)) |
There was a problem hiding this comment.
doesn't this have to come after the flag.Parse call though?
There was a problem hiding this comment.
It definitely should if the logLevel specified mattered. But as you said in the commented issue #4055, it doesn't actually affect the log levels until they're set later on.
I agree, it should probably go after the version check.
current versions like 4.6.0 have extra ssl/https checks our graphite has a self signed ssl cert, but not for the hostname 'graphitemon' that we give the container, so grafana would complain. I tried to use https://github.com/graphite-project/docker-graphite-statsd instead (which basically just requires switching the port to 80, and updating the config files) but i couldn't get any data into it, so gave up on that.
note: `out` is copied from fakemetrics. the reason we had to do that is because otherwise go compiler would complain about MetricData from schema package in MT's vendored dir vs that of fakemetrics not being the same. Hopefully we can clean that up later.
and remove redundant checks
seeing an uninitialized response could be confusing so use pointer, for which we use spew to show it nicely
This way we can use the latest grafana which has built-in annotations (see next commit) see also a7332fd carbon config is same as default, minus all the comments, and just these changes: -MAX_UPDATES_PER_SECOND = 500 -MAX_CREATES_PER_MINUTE = 50 +MAX_UPDATES_PER_SECOND = 5000 +MAX_CREATES_PER_MINUTE = 5000 -CACHE_WRITE_STRATEGY = sorted +CACHE_WRITE_STRATEGY = max
(see validateCorrect comments)
because the OK's and subsequent test starts are so close together they are indiscernable on charts and it looks confusing.
e.g. in a case like 3 [5 4 4 4 4 4 4] the average would clearly have been 4
so we can easily categorize output logs based on which test
Using the logger before you initialize it results in nothing. It would never print out `log.Fatal(4, "error with configuration file: %s", err)` and merely exit After parsing, change to the user specified log level
modifies the index loading so only names/tagSets are pruned where all MetricDefinitions are Stale. fixes grafana#813
fixes issue grafana#727
If users want to build their own backend stores they need to be able to process mdata.ChunkWriteRequest payloads. TO be able to do this, the fields of the ChunkWriteRequest need to be exported.
|
Lmao that didn't work. |
as of #823/#825, the logger was created before flags were parsed and the config (file and env vars) were loaded. so the logger would always get the default value for the flag and no way to change the level.. I tried creating the logger after flag parsing, and adjusting the log level after config parsing, but couldn't the second step to work properly. Simplest solution is to just print to stderr directly the one time we actually need it in between the two steps. PS: I hate this logging library, but we'll get rid of it. see #624
The project attempts to use the logger before it's initialized.
For example, it would never print out log.Fatal(4, "error with configuration file: %s", err) and merely exit. Additionally the logger is used in loading configs meaning they silently fail.
The user specified logging levels are applied after configs are parsed.