dynamic config: enhance update with metrics etc#9366
dynamic config: enhance update with metrics etc#9366feiyang3cat wants to merge 2 commits intotemporalio:mainfrom
Conversation
feiyang3cat
changed the title
| fc.lastUpdatedTime = modtime | ||
| defer func() { | ||
| if updateErr != nil { | ||
| fc.lastUpdatedTime = prevModtime |
| tag.Bool("debug-mode", debug.Enabled), | ||
| ) | ||
|
|
||
| metricsHandler, err := metrics.MetricsHandlerFromConfig(logger, cfg.Global.Metrics) |
There was a problem hiding this comment.
This is kind of unfortunate. I would rather see main() use fewer ServerOptions and rely on the defaults more, instead of the opposite. Do you think you could look into what it would take to move initialization of both metrics handler and dc client out of main and use the defaults?
There was a problem hiding this comment.
Definitely! I'd love to look into it. how about I create another Jira task for this separately ? so that this refinement doesn't impact an incident fix
| fc.lastUpdatedTime = modtime | ||
| defer func() { | ||
| if updateErr != nil { | ||
| fc.lastUpdatedTime = prevModtime |
There was a problem hiding this comment.
this would be cleaner if it updated forward in the defer in the no-error case
| updateErr = fmt.Errorf("dynamic config file: %s: %w", fc.config.Filepath, err) | ||
| return |
There was a problem hiding this comment.
you don't have the change this, the one-line return works fine even if you have a named return
feiyang3cat
force-pushed
the
dc/update-metric
branch
from
36c5a8f to
36d68a2
Compare
feiyang3cat
force-pushed
the
dc/update-metric
branch
from
36d68a2 to
eb7d16c
Compare
2 participants
What changed?
Why?
may be a root cause why pods run into crashloop
adding this metric for alert make debugging easier
How did you test it?