fix(backend): log messages correctly with json and timestamps

Fixes #3822

CHANGELOG.md

@@ -1,5 +1,17 @@
 # Changelog
 
+## [next]
+
+### Fixed
+
+- Messages are now logged correctly when both `--log.format=json` and
+  `--log.timestamp=true` flags are set #3822.
+
+### Changed
+
+- Show a placeholder message if no alertmanager server is configured, instead of
+  failing to start.
+
 ## v0.94
 
 ### Added

cmd/karma/alerts.go

@@ -84,7 +84,9 @@ func countersToLabelStats(counters map[string]map[string]int) models.LabelNameSt
 }
 
 func getUpstreams() models.AlertmanagerAPISummary {
-	summary := models.AlertmanagerAPISummary{}
+	summary := models.AlertmanagerAPISummary{
+		Instances: []models.AlertmanagerAPIStatus{},
+	}
 
 	clusters := map[string][]string{}
 	upstreams := alertmanager.GetAlertmanagers()

cmd/karma/main.go

@@ -254,6 +254,10 @@ func lvlFormatter(level interface{}) string {
 	return fmt.Sprintf("level=%s", level)
 }
 
+func discardFormatter(msg interface{}) string {
+	return ""
+}
+
 func initLogger() {
 	log.Logger = log.Logger.Output(zerolog.ConsoleWriter{
 		Out:           os.Stderr,
@@ -279,9 +283,19 @@ func setupLogger() error {
 				FormatMessage: msgFormatter,
 				TimeFormat:    "15:04:05",
 			})
+		} else {
+			log.Logger = zerolog.New(os.Stderr).Output(zerolog.ConsoleWriter{
+				Out:             os.Stderr,
+				NoColor:         true,
+				FormatLevel:     lvlFormatter,
+				FormatMessage:   msgFormatter,
+				FormatTimestamp: discardFormatter,
+			})
 		}
 	case "json":
-		if !config.Config.Log.Timestamp {
+		if config.Config.Log.Timestamp {
+			log.Logger = zerolog.New(os.Stderr).With().Timestamp().Logger()
+		} else {
 			log.Logger = zerolog.New(os.Stderr).With().Logger()
 		}
 	default:
@@ -382,10 +396,6 @@ func mainSetup(errorHandling pflag.ErrorHandling) (*chi.Mux, *historyPoller, err
 		return nil, nil, err
 	}
 
-	if len(alertmanager.GetAlertmanagers()) == 0 {
-		return nil, nil, fmt.Errorf("no valid Alertmanager URIs defined")
-	}
-
 	if config.Config.Authorization.ACL.Silences != "" {
 		log.Info().
 			Str("path", config.Config.Authorization.ACL.Silences).

cmd/karma/tests/testscript/058_log_format_json.txt

@@ -1,8 +1,22 @@
 # Logs messages as JSON when log.format=json is passed
-karma.bin-should-fail --log.format=json --log.timestamp=false
+
+exec bash -x ./test.sh &
+karma.bin-should-work --log.format=json --log.timestamp=false --pid-file=karma.pid --listen.address=127.0.0.1 --listen.port=8058
 ! stdout .
 cmp stderr stderr.txt
 
 -- stderr.txt --
 {"level":"info","message":"Version: dev"}
-{"level":"error","error":"no valid Alertmanager URIs defined","message":"Execution failed"}
+{"level":"info","path":"karma.pid","message":"Writing PID file"}
+{"level":"info","message":"Initial Alertmanager collection"}
+{"level":"info","message":"Pulling latest alerts and silences from Alertmanager"}
+{"level":"info","message":"Collection completed"}
+{"level":"info","message":"Done, starting HTTP server"}
+{"level":"info","address":"127.0.0.1:8058","message":"Starting HTTP server"}
+{"level":"info","message":"Shutting down HTTP server"}
+{"level":"info","message":"HTTP server shut down"}
+{"level":"info","path":"karma.pid","message":"Removing PID file"}
+-- test.sh --
+while [ ! -f karma.pid ]; do sleep 1 ; done
+sleep 5
+cat karma.pid | xargs kill

cmd/karma/tests/testscript/062_log_timestamp.txt

@@ -1,5 +1,12 @@
 # Logs timestamps when log.timestamp is set to true
-karma.bin-should-fail --log.timestamp=true
+
+exec bash -x ./test.sh &
+karma.bin-should-work --log.timestamp=true --pid-file=karma.pid --listen.address=127.0.0.1 --listen.port=8062
 ! stdout .
 stderr '[0-9][0-9]:[0-9][0-9]:[0-9][0-9] level=info msg="Version: dev"'
-stderr '[0-9][0-9]:[0-9][0-9]:[0-9][0-9] level=error msg="Execution failed" error="no valid Alertmanager URIs defined"'
+stderr '[0-9][0-9]:[0-9][0-9]:[0-9][0-9] level=info msg="Pulling latest alerts and silences from Alertmanager"'
+
+-- test.sh --
+while [ ! -f karma.pid ]; do sleep 1 ; done
+sleep 5
+cat karma.pid | xargs kill

cmd/karma/tests/testscript/064_log_timestamp_json.txt

@@ -0,0 +1,12 @@
+# Logs messages as JSON when log.format=json & log.timestamp=true is passed
+
+exec bash -x ./test.sh &
+karma.bin-should-work --log.format=json --log.timestamp=true --pid-file=karma.pid --listen.address=127.0.0.1 --listen.port=8064
+! stdout .
+stderr '{"level":"info","time":"[0-9]+-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+Z","message":"Version: dev"}'
+stderr '{"level":"info","time":"[0-9]+-[0-9]+-[0-9]+T[0-9]+:[0-9]+:[0-9]+Z","message":"Pulling latest alerts and silences from Alertmanager"}'
+
+-- test.sh --
+while [ ! -f karma.pid ]; do sleep 1 ; done
+sleep 5
+cat karma.pid | xargs kill

cmd/karma/tests/testscript/064_no_args.txt

@@ -1,8 +0,0 @@
-# Raises an error if no alertmanager uri is set
-karma.bin-should-fail
-! stdout .
-cmp stderr stderr.txt
-
--- stderr.txt --
-level=info msg="Version: dev"
-level=error msg="Execution failed" error="no valid Alertmanager URIs defined"

ui/src/Components/Grid/NoUpstream/__snapshots__/index.test.tsx.snap

@@ -0,0 +1,9 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`<NoUpstream /> matches snapshot 1`] = `
+"
+<h1 class=\\"display-1 text-placeholder screen-center\\">
+  No alertmanager server configured
+</h1>
+"
+`;

ui/src/Components/Grid/NoUpstream/index.test.tsx

@@ -0,0 +1,19 @@
+import React from "react";
+
+import { shallow } from "enzyme";
+
+import toDiffableHtml from "diffable-html";
+
+import { MockThemeContext } from "__fixtures__/Theme";
+import { NoUpstream } from ".";
+
+beforeEach(() => {
+  jest.spyOn(React, "useContext").mockImplementation(() => MockThemeContext);
+});
+
+describe("<NoUpstream />", () => {
+  it("matches snapshot", () => {
+    const tree = shallow(<NoUpstream />);
+    expect(toDiffableHtml(tree.html())).toMatchSnapshot();
+  });
+});

ui/src/Components/Grid/NoUpstream/index.tsx

@@ -0,0 +1,9 @@
+import type { FC } from "react";
+
+import { CenteredMessage } from "Components/CenteredMessage";
+
+const NoUpstream: FC = () => (
+  <CenteredMessage>No alertmanager server configured</CenteredMessage>
+);
+
+export { NoUpstream };

ui/src/Components/Grid/index.test.tsx

@@ -131,10 +131,40 @@ describe("<Grid />", () => {
   it("renders EmptyGrid after first fetch when totalAlerts is 0", () => {
     alertStore.info.setVersion("1.2.3");
     alertStore.info.setTotalAlerts(0);
+    alertStore.data.setUpstreams({
+      counters: { total: 1, healthy: 1, failed: 1 },
+      instances: [
+        {
+          name: "dev",
+          cluster: "dev",
+          clusterMembers: ["dev"],
+          uri: "https://am.example.com",
+          publicURI: "https://am.example.com",
+          error: "",
+          readonly: false,
+          headers: {},
+          corsCredentials: "include",
+          version: "",
+        },
+      ],
+      clusters: { dev: ["dev"] },
+    });
     const tree = ShallowGrid();
     expect(tree.text()).toBe("<EmptyGrid />");
   });
 
+  it("renders NoUpstream after first fetch when upstream list is empty", () => {
+    alertStore.info.setVersion("1.2.3");
+    alertStore.info.setTotalAlerts(0);
+    alertStore.data.setUpstreams({
+      counters: { total: 0, healthy: 0, failed: 0 },
+      instances: [],
+      clusters: {},
+    });
+    const tree = ShallowGrid();
+    expect(tree.text()).toBe("<NoUpstream />");
+  });
+
   it("renders AlertGrid after first fetch finished when totalAlerts is >0", () => {
     alertStore.info.setVersion("unknown");
     alertStore.info.setTotalAlerts(1);

ui/src/Components/Grid/index.tsx

@@ -10,6 +10,7 @@ import { FatalError } from "./FatalError";
 import { UpgradeNeeded } from "./UpgradeNeeded";
 import { ReloadNeeded } from "./ReloadNeeded";
 import { EmptyGrid } from "./EmptyGrid";
+import { NoUpstream } from "./NoUpstream";
 
 const Grid: FC<{
   alertStore: AlertStore;
@@ -30,7 +31,11 @@ const Grid: FC<{
     <FatalError message={alertStore.data.upstreams.instances[0].error} />
   ) : alertStore.info.version !== "unknown" &&
     alertStore.info.totalAlerts === 0 ? (
-    <EmptyGrid />
+    alertStore.data.upstreams.instances.length === 0 ? (
+      <NoUpstream />
+    ) : (
+      <EmptyGrid />
+    )
   ) : (
     <AlertGrid
       alertStore={alertStore}

ui/src/Components/Modal/index.test.tsx

@@ -4,6 +4,11 @@ import { act } from "react-dom/test-utils";
 import { mount } from "enzyme";
 
 import { PressKey } from "__fixtures__/PressKey";
+import {
+  MockThemeContext,
+  MockThemeContextWithoutAnimations,
+} from "__fixtures__/Theme";
+import { ThemeContext } from "Components/Theme";
 import { Modal, ModalInner } from ".";
 
 beforeEach(() => {
@@ -116,6 +121,38 @@ describe("<ModalInner />", () => {
     expect((mountModal.props() as any).onExited).toBe(onExited);
   });
 
+  it("uses components-animation-modal class when animations are enabled", () => {
+    const onExited = jest.fn();
+    const tree = mount(
+      <Modal isOpen={true} toggleOpen={fakeToggle} onExited={onExited}>
+        <div />
+      </Modal>,
+      {
+        wrappingComponent: ThemeContext.Provider,
+        wrappingComponentProps: { value: MockThemeContext },
+      }
+    );
+    const mountModal = tree.find("CSSTransition").at(0);
+    expect((mountModal.props() as any).classNames).toBe(
+      "components-animation-modal"
+    );
+  });
+
+  it("doesn't use components-animation-modal class when animations are disabled", () => {
+    const onExited = jest.fn();
+    const tree = mount(
+      <Modal isOpen={true} toggleOpen={fakeToggle} onExited={onExited}>
+        <div />
+      </Modal>,
+      {
+        wrappingComponent: ThemeContext.Provider,
+        wrappingComponentProps: { value: MockThemeContextWithoutAnimations },
+      }
+    );
+    const mountModal = tree.find("CSSTransition").at(0);
+    expect((mountModal.props() as any).classNames).toBe("");
+  });
+
   it("toggleOpen is called after pressing 'esc'", () => {
     MountedModal(true);
     PressKey("Escape", 27);

ui/src/Components/NavBar/index.stories.tsx

@@ -36,6 +36,24 @@ storiesOf("NavBar", module).add("NavBar", () => {
   const settingsStore = new Settings(null);
   const silenceFormStore = new SilenceFormStore();
 
+  alertStore.data.setUpstreams({
+    counters: { total: 1, healthy: 1, failed: 0 },
+    instances: [
+      {
+        name: "dev",
+        cluster: "dev",
+        clusterMembers: ["dev"],
+        uri: "https://am.example.com",
+        publicURI: "https://am.example.com",
+        error: "",
+        readonly: false,
+        headers: {},
+        corsCredentials: "include",
+        version: "",
+      },
+    ],
+    clusters: { dev: ["dev"] },
+  });
   alertStore.info.setTotalAlerts(197);
   alertStore.data.setColors({
     cluster: {

ui/src/Components/NavBar/index.test.tsx

@@ -40,6 +40,25 @@ beforeEach(() => {
     cb(0);
     return 0;
   });
+
+  alertStore.data.setUpstreams({
+    counters: { total: 1, healthy: 1, failed: 0 },
+    instances: [
+      {
+        name: "dev",
+        cluster: "dev",
+        clusterMembers: ["dev"],
+        uri: "https://am.example.com",
+        publicURI: "https://am.example.com",
+        error: "",
+        readonly: false,
+        headers: {},
+        corsCredentials: "include",
+        version: "",
+      },
+    ],
+    clusters: { dev: ["dev"] },
+  });
 });
 
 afterEach(() => {
@@ -64,6 +83,17 @@ const MountedNavbar = (fixedTop?: boolean) => {
 };
 
 describe("<NavBar />", () => {
+  it("renders null with no upstreams", () => {
+    alertStore.data.setUpstreams({
+      counters: { total: 0, healthy: 0, failed: 0 },
+      instances: [],
+      clusters: {},
+    });
+    alertStore.info.setTotalAlerts(15);
+    const tree = MountedNavbar();
+    expect(tree.find("span.navbar-brand")).toHaveLength(0);
+  });
+
   it("navbar-brand shows 15 alerts with totalAlerts=15", () => {
     alertStore.info.setTotalAlerts(15);
     const tree = MountedNavbar();

ui/src/Components/NavBar/index.tsx

@@ -117,21 +117,33 @@ const NavBar: FC<{
           } align-items-start`}
         >
           <span className="navbar-nav d-flex flex-row">
-            <span className="navbar-brand p-0 my-0 mx-2 h1 d-none d-sm-block">
-              <OverviewModal alertStore={alertStore} />
-            </span>
+            {alertStore.data.upstreams.instances.length === 0 ? null : (
+              <span className="navbar-brand p-0 my-0 mx-2 h1 d-none d-sm-block">
+                <OverviewModal alertStore={alertStore} />
+              </span>
+            )}
             <Fetcher alertStore={alertStore} settingsStore={settingsStore} />
           </span>
-          <FilterInput alertStore={alertStore} settingsStore={settingsStore} />
-          <ul className="navbar-nav flex-wrap flex-shrink-1 ms-1">
-            <AppToasts alertStore={alertStore} />
-            <SilenceModal
+          {alertStore.data.upstreams.instances.length === 0 ? null : (
+            <FilterInput
               alertStore={alertStore}
-              silenceFormStore={silenceFormStore}
               settingsStore={settingsStore}
             />
-            <MainModal alertStore={alertStore} settingsStore={settingsStore} />
-          </ul>
+          )}
+          {alertStore.data.upstreams.instances.length === 0 ? null : (
+            <ul className="navbar-nav flex-wrap flex-shrink-1 ms-1">
+              <AppToasts alertStore={alertStore} />
+              <SilenceModal
+                alertStore={alertStore}
+                silenceFormStore={silenceFormStore}
+                settingsStore={settingsStore}
+              />
+              <MainModal
+                alertStore={alertStore}
+                settingsStore={settingsStore}
+              />
+            </ul>
+          )}
         </nav>
       </CSSTransition>
     </div>