You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2021/08/18 21:21:09 UTC

[GitHub] [trafficcontrol] ocket8888 opened a new pull request #6120: TP tests alert logging

ocket8888 opened a new pull request #6120:
URL: https://github.com/apache/trafficcontrol/pull/6120


   This PR adds logging `alerts` to the console for any and all raw API requests made by the Traffic Portal integration tests. So that if, e.g. the Roles your TO instance have don't match the expected hard-coded id-to-Role mappings of the tests, you'll be able to see an error that says `no such role` in the console.
   
   This also cleans up the GHA logs by moving TO, TV, and TP logs into build artifacts that only get uploaded on failure. This makes it **much** easier to follow testing output and see what went wrong, and the TO/TV logs are less necessary to understand why tests fail now that response alerts are being logged.
   
   <!-- **^ Add meaningful description above** --><hr>
   
   ## Which Traffic Control components are affected by this PR?
   - Traffic Portal (integration tests)
   - Automation (GitHub Actions)
   
   ## What is the best way to verify this PR?
   Make sure the test actions pass. To see an example of the alert logging, you could make a branch based on this changeset, comment out the line in Traffic Ops's `routes.go` that adds the `/api/4.0/user/login` endpoint (API version there matters; the tests use 4.0) and push it to your remote. Then you should see a line very near the bottom after between 2 and 4 minutes when the action fails that says:
   ```
   ERROR: POST https://172.18.0.1:8443/api/4.0/user/login (404 Not Found): The requested path '/api/4.0/user/login' does not exist.
   ```
   
   ## PR submission checklist
   - [x] This PR has tests <!-- If not, please delete this text and explain why this PR does not need tests. -->
   - [ ] This PR has no documentation because it doesn't change the existing behavior of any component (testing configuration changes are also non-breaking for valid configurations)
   - [ ] This PR has no CHANGELOG.md entry because it doesn't change the existing behavior of any component
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY**


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6120: TP tests alert logging

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6120:
URL: https://github.com/apache/trafficcontrol/pull/6120#discussion_r692962269



##########
File path: .github/actions/tp-integration-tests/cdn.json
##########
@@ -13,11 +13,11 @@
 	"traffic_ops_golang": {
 		"insecure": true,
 		"port": "6443",
-		"log_location_error": "error.log",
-		"log_location_warning": "warning.log",
-		"log_location_info": "info.log",
-		"log_location_debug": null,
-		"log_location_event": "event.log",
+		"log_location_error": "stderr",
+		"log_location_warning": "stderr",
+		"log_location_info": "stdout",
+		"log_location_debug": "stderr",

Review comment:
       eh, sure. It doesn't make much difference, honestly maybe they should all just be the same stream. Idk, I'll just make it stdout for now.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6120: TP tests alert logging

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6120:
URL: https://github.com/apache/trafficcontrol/pull/6120#discussion_r692316859



##########
File path: .github/actions/tp-integration-tests/cdn.json
##########
@@ -13,11 +13,11 @@
 	"traffic_ops_golang": {
 		"insecure": true,
 		"port": "6443",
-		"log_location_error": "error.log",
-		"log_location_warning": "warning.log",
-		"log_location_info": "info.log",
-		"log_location_debug": null,
-		"log_location_event": "event.log",
+		"log_location_error": "stderr",
+		"log_location_warning": "stderr",
+		"log_location_info": "stdout",
+		"log_location_debug": "stderr",

Review comment:
       or did you mean like "should this _really_ be stdout instead of stderr?" Or were you talking about "log_location_debug"? There was a point when "log_location_info" was set to just "out" instead of "stdout", so I assumed you made this comment before I fixed that typo, but now that I think about it I'm not sure.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficcontrol] shamrickus commented on a change in pull request #6120: TP tests alert logging

Posted by GitBox <gi...@apache.org>.
shamrickus commented on a change in pull request #6120:
URL: https://github.com/apache/trafficcontrol/pull/6120#discussion_r692321521



##########
File path: traffic_portal/test/integration/config.model.ts
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { hasProperty } from "./CommonUtils/utils";
+
+/** Possible levels of TO Alerts */
+export type AlertLevel = "success" | "info" | "warning" | "error";
+
+/**
+ * Checks whether an object is a valid Alert level.
+ *
+ * @param s The object to check.
+ * @returns Whether or not `s` is an AlertLevel.
+ */
+export function isAlertLevel(s: unknown): s is AlertLevel {
+    if (typeof(s) !== "string") {
+        return false;
+    }
+    switch (s) {
+        case "success":
+        case "info":
+        case "warning":
+        case "error":
+            return true;
+    }
+    return false;
+}
+
+/** TO API Alerts */
+export interface Alert {
+    level: AlertLevel;
+    text: string;
+}
+
+/**
+ * Checks whether an object is an Alert like the ones TO normally returns.
+ *
+ * @param a The object to check.
+ * @returns Whether or not `a` is an Alert.
+ */
+export function isAlert(a: unknown): a is Alert {
+    if (typeof(a) !== "object" || a === null) {
+        return false;
+    }
+    if (!hasProperty(a, "level") || !hasProperty(a, "text", "string")) {
+        return false;
+    }
+    return isAlertLevel(a.level);
+}
+
+/**
+ * Logs an alert to the appropriate console stream based on its `level`.
+ *
+ * @param a The Alert to log.
+ * @param prefix Optional prefix for the log message
+ */
+export function logAlert(a: Alert, prefix?: string): void {
+    let logfunc;
+    let pre = (prefix ?? "").trimStart();
+    switch (a.level) {
+        case "success":
+            logfunc = console.log;
+            pre = `SUCCESS: ${pre}`;
+            break;
+        case "info":
+            logfunc = console.info
+            pre = `INFO: ${pre}`;
+            break;
+        case "warning":
+            logfunc = console.warn
+            pre = `WARN: ${pre}`;
+            break;
+        case "error":
+            logfunc = console.error
+            pre = `ERROR: ${pre}`;
+            break;
+    }
+    logfunc(pre, a.text);
+}
+
+/** TestingConfig is the type of a testing configuration. */
+export interface TestingConfig {
+	/** This is login information for a user with admin-level permissions. */
+	readonly login: {
+		readonly password: string;
+		readonly username: string;
+	};
+	/** The URL at which the Traffic Ops API can be accessed. */
+	readonly apiUrl: string;
+	/** The URL at which Traffic Portal is served - root path. */
+	readonly baseUrl: string;
+	/** Logging alert levels that are enabled. */
+	readonly alertLevels?: Array<AlertLevel>;
+}
+
+/**
+ * Checks if a passed object is a valid testing configuration.
+ *
+ * @param c The object to check.
+ * @returns `true`, always. When the check fails, it throws an error that
+ * explains why.
+ */
+export function isTestingConfig(c: unknown): c is TestingConfig {
+	if (typeof(c) !== "object") {
+		throw new Error(`testing configuration must be an object, not a '${typeof(c)}'`);
+	}
+	if (c === null) {

Review comment:
       Fair point




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficcontrol] shamrickus commented on a change in pull request #6120: TP tests alert logging

Posted by GitBox <gi...@apache.org>.
shamrickus commented on a change in pull request #6120:
URL: https://github.com/apache/trafficcontrol/pull/6120#discussion_r692205667



##########
File path: .github/actions/tp-integration-tests/cdn.json
##########
@@ -13,11 +13,11 @@
 	"traffic_ops_golang": {
 		"insecure": true,
 		"port": "6443",
-		"log_location_error": "error.log",
-		"log_location_warning": "warning.log",
-		"log_location_info": "info.log",
-		"log_location_debug": null,
-		"log_location_event": "event.log",
+		"log_location_error": "stderr",
+		"log_location_warning": "stderr",
+		"log_location_info": "stdout",
+		"log_location_debug": "stderr",

Review comment:
       Should this be `stdout`?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6120: TP tests alert logging

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6120:
URL: https://github.com/apache/trafficcontrol/pull/6120#discussion_r692309978



##########
File path: traffic_portal/test/integration/config.model.ts
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { hasProperty } from "./CommonUtils/utils";
+
+/** Possible levels of TO Alerts */
+export type AlertLevel = "success" | "info" | "warning" | "error";
+
+/**
+ * Checks whether an object is a valid Alert level.
+ *
+ * @param s The object to check.
+ * @returns Whether or not `s` is an AlertLevel.
+ */
+export function isAlertLevel(s: unknown): s is AlertLevel {
+    if (typeof(s) !== "string") {
+        return false;
+    }
+    switch (s) {
+        case "success":
+        case "info":
+        case "warning":
+        case "error":
+            return true;
+    }
+    return false;
+}
+
+/** TO API Alerts */
+export interface Alert {
+    level: AlertLevel;
+    text: string;
+}
+
+/**
+ * Checks whether an object is an Alert like the ones TO normally returns.
+ *
+ * @param a The object to check.
+ * @returns Whether or not `a` is an Alert.
+ */
+export function isAlert(a: unknown): a is Alert {
+    if (typeof(a) !== "object" || a === null) {
+        return false;
+    }
+    if (!hasProperty(a, "level") || !hasProperty(a, "text", "string")) {
+        return false;
+    }
+    return isAlertLevel(a.level);
+}
+
+/**
+ * Logs an alert to the appropriate console stream based on its `level`.
+ *
+ * @param a The Alert to log.
+ * @param prefix Optional prefix for the log message
+ */
+export function logAlert(a: Alert, prefix?: string): void {
+    let logfunc;
+    let pre = (prefix ?? "").trimStart();
+    switch (a.level) {
+        case "success":
+            logfunc = console.log;
+            pre = `SUCCESS: ${pre}`;
+            break;
+        case "info":
+            logfunc = console.info
+            pre = `INFO: ${pre}`;
+            break;
+        case "warning":
+            logfunc = console.warn
+            pre = `WARN: ${pre}`;
+            break;
+        case "error":
+            logfunc = console.error
+            pre = `ERROR: ${pre}`;
+            break;
+    }
+    logfunc(pre, a.text);
+}
+
+/** TestingConfig is the type of a testing configuration. */
+export interface TestingConfig {
+	/** This is login information for a user with admin-level permissions. */
+	readonly login: {
+		readonly password: string;
+		readonly username: string;
+	};
+	/** The URL at which the Traffic Ops API can be accessed. */
+	readonly apiUrl: string;
+	/** The URL at which Traffic Portal is served - root path. */
+	readonly baseUrl: string;
+	/** Logging alert levels that are enabled. */
+	readonly alertLevels?: Array<AlertLevel>;
+}
+
+/**
+ * Checks if a passed object is a valid testing configuration.
+ *
+ * @param c The object to check.
+ * @returns `true`, always. When the check fails, it throws an error that
+ * explains why.
+ */
+export function isTestingConfig(c: unknown): c is TestingConfig {
+	if (typeof(c) !== "object") {
+		throw new Error(`testing configuration must be an object, not a '${typeof(c)}'`);
+	}
+	if (c === null) {

Review comment:
       Yeah, but `typeof` before null check is marginally faster in case of anything that isn't `null` or an object, which is more things than just `null`. I think it'd be more common for the configuration to have a typo or be incomplete so that the input value is actually `undefined` like e.g.:
   ```json
   {
       "arams": {
           "config": "options here"
       }
   }
   ```
   ... than it would be for it to be like:
   ```json
   {
       "params": null
   }
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficcontrol] shamrickus commented on a change in pull request #6120: TP tests alert logging

Posted by GitBox <gi...@apache.org>.
shamrickus commented on a change in pull request #6120:
URL: https://github.com/apache/trafficcontrol/pull/6120#discussion_r692204233



##########
File path: .github/actions/tp-integration-tests/entrypoint.sh
##########
@@ -167,56 +182,42 @@ to_build() {
 
   export $(<"${ciab_dir}/variables.env" sed '/^#/d') # defines TV_ADMIN_USER/PASSWORD
   envsubst <"${resources}/riak.json" >riak.conf
-  truncate --size=0 warning.log error.log event.log info.log
+  truncate -s0 out.log
 
-  ./traffic_ops_golang --cfg ./cdn.conf --dbcfg ./database.conf -riakcfg riak.conf &
-  tail -f warning.log 2>&1 | color_and_prefix "${yellow_bg}" 'Traffic Ops WARN' &
-  tail -f error.log 2>&1 | color_and_prefix "${red_bg}" 'Traffic Ops ERR' &
-  tail -f event.log 2>&1 | color_and_prefix "${gray_bg}" 'Traffic Ops EVT' &
+  ./traffic_ops_golang --cfg ./cdn.conf --dbcfg ./database.conf -riakcfg riak.conf >out.log 2>&1 &
+  popd
 }
 
 tp_build() {
-  cd "${REPO_DIR}/traffic_portal"
+  pushd "${REPO_DIR}/traffic_portal"
   npm ci
   bower install
   grunt dist
 
   cp "${resources}/config.js" ./conf/
   touch tp.log access.log out.log err.log
-  sudo forever --minUptime 5000 --spinSleepTime 2000 -f -o out.log start server.js &
-  tail -f err.log 2>&1 | color_and_prefix "${red_bg}" "Node Err" &
+  sudo forever --minUptime 5000 --spinSleepTime 2000 -f -o out.log start server.js >out.log 2>&1 &

Review comment:
       `-o out.log` is redundant.

##########
File path: .github/actions/tp-integration-tests/cdn.json
##########
@@ -13,11 +13,11 @@
 	"traffic_ops_golang": {
 		"insecure": true,
 		"port": "6443",
-		"log_location_error": "error.log",
-		"log_location_warning": "warning.log",
-		"log_location_info": "info.log",
-		"log_location_debug": null,
-		"log_location_event": "event.log",
+		"log_location_error": "stderr",
+		"log_location_warning": "stderr",
+		"log_location_info": "stdout",
+		"log_location_debug": "stderr",

Review comment:
       Shouldn't this be `stdout`?

##########
File path: traffic_portal/test/integration/config.model.ts
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { hasProperty } from "./CommonUtils/utils";
+
+/** Possible levels of TO Alerts */
+export type AlertLevel = "success" | "info" | "warning" | "error";
+
+/**
+ * Checks whether an object is a valid Alert level.
+ *
+ * @param s The object to check.
+ * @returns Whether or not `s` is an AlertLevel.
+ */
+export function isAlertLevel(s: unknown): s is AlertLevel {
+    if (typeof(s) !== "string") {
+        return false;
+    }
+    switch (s) {
+        case "success":
+        case "info":
+        case "warning":
+        case "error":
+            return true;
+    }
+    return false;
+}
+
+/** TO API Alerts */
+export interface Alert {
+    level: AlertLevel;
+    text: string;
+}
+
+/**
+ * Checks whether an object is an Alert like the ones TO normally returns.
+ *
+ * @param a The object to check.
+ * @returns Whether or not `a` is an Alert.
+ */
+export function isAlert(a: unknown): a is Alert {
+    if (typeof(a) !== "object" || a === null) {
+        return false;
+    }
+    if (!hasProperty(a, "level") || !hasProperty(a, "text", "string")) {
+        return false;
+    }
+    return isAlertLevel(a.level);
+}
+
+/**
+ * Logs an alert to the appropriate console stream based on its `level`.
+ *
+ * @param a The Alert to log.
+ * @param prefix Optional prefix for the log message
+ */
+export function logAlert(a: Alert, prefix?: string): void {
+    let logfunc;
+    let pre = (prefix ?? "").trimStart();
+    switch (a.level) {
+        case "success":
+            logfunc = console.log;
+            pre = `SUCCESS: ${pre}`;
+            break;
+        case "info":
+            logfunc = console.info
+            pre = `INFO: ${pre}`;
+            break;
+        case "warning":
+            logfunc = console.warn
+            pre = `WARN: ${pre}`;
+            break;
+        case "error":
+            logfunc = console.error
+            pre = `ERROR: ${pre}`;
+            break;
+    }
+    logfunc(pre, a.text);
+}
+
+/** TestingConfig is the type of a testing configuration. */
+export interface TestingConfig {
+	/** This is login information for a user with admin-level permissions. */
+	readonly login: {
+		readonly password: string;
+		readonly username: string;
+	};
+	/** The URL at which the Traffic Ops API can be accessed. */
+	readonly apiUrl: string;
+	/** The URL at which Traffic Portal is served - root path. */
+	readonly baseUrl: string;
+	/** Logging alert levels that are enabled. */
+	readonly alertLevels?: Array<AlertLevel>;
+}
+
+/**
+ * Checks if a passed object is a valid testing configuration.
+ *
+ * @param c The object to check.
+ * @returns `true`, always. When the check fails, it throws an error that
+ * explains why.
+ */
+export function isTestingConfig(c: unknown): c is TestingConfig {
+	if (typeof(c) !== "object") {
+		throw new Error(`testing configuration must be an object, not a '${typeof(c)}'`);
+	}
+	if (c === null) {

Review comment:
       nit: null check before `typeof` is marginally faster in case of nulls
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficcontrol] zrhoffman merged pull request #6120: TP tests alert logging

Posted by GitBox <gi...@apache.org>.
zrhoffman merged pull request #6120:
URL: https://github.com/apache/trafficcontrol/pull/6120


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficcontrol] shamrickus commented on a change in pull request #6120: TP tests alert logging

Posted by GitBox <gi...@apache.org>.
shamrickus commented on a change in pull request #6120:
URL: https://github.com/apache/trafficcontrol/pull/6120#discussion_r692320934



##########
File path: .github/actions/tp-integration-tests/cdn.json
##########
@@ -13,11 +13,11 @@
 	"traffic_ops_golang": {
 		"insecure": true,
 		"port": "6443",
-		"log_location_error": "error.log",
-		"log_location_warning": "warning.log",
-		"log_location_info": "info.log",
-		"log_location_debug": null,
-		"log_location_event": "event.log",
+		"log_location_error": "stderr",
+		"log_location_warning": "stderr",
+		"log_location_info": "stdout",
+		"log_location_debug": "stderr",

Review comment:
       I meant since info is stdout shouldn't debug also be stdout




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6120: TP tests alert logging

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6120:
URL: https://github.com/apache/trafficcontrol/pull/6120#discussion_r692304329



##########
File path: .github/actions/tp-integration-tests/cdn.json
##########
@@ -13,11 +13,11 @@
 	"traffic_ops_golang": {
 		"insecure": true,
 		"port": "6443",
-		"log_location_error": "error.log",
-		"log_location_warning": "warning.log",
-		"log_location_info": "info.log",
-		"log_location_debug": null,
-		"log_location_event": "event.log",
+		"log_location_error": "stderr",
+		"log_location_warning": "stderr",
+		"log_location_info": "stdout",
+		"log_location_debug": "stderr",

Review comment:
       It is




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficcontrol] ocket8888 commented on a change in pull request #6120: TP tests alert logging

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6120:
URL: https://github.com/apache/trafficcontrol/pull/6120#discussion_r692307845



##########
File path: .github/actions/tp-integration-tests/entrypoint.sh
##########
@@ -167,56 +182,42 @@ to_build() {
 
   export $(<"${ciab_dir}/variables.env" sed '/^#/d') # defines TV_ADMIN_USER/PASSWORD
   envsubst <"${resources}/riak.json" >riak.conf
-  truncate --size=0 warning.log error.log event.log info.log
+  truncate -s0 out.log
 
-  ./traffic_ops_golang --cfg ./cdn.conf --dbcfg ./database.conf -riakcfg riak.conf &
-  tail -f warning.log 2>&1 | color_and_prefix "${yellow_bg}" 'Traffic Ops WARN' &
-  tail -f error.log 2>&1 | color_and_prefix "${red_bg}" 'Traffic Ops ERR' &
-  tail -f event.log 2>&1 | color_and_prefix "${gray_bg}" 'Traffic Ops EVT' &
+  ./traffic_ops_golang --cfg ./cdn.conf --dbcfg ./database.conf -riakcfg riak.conf >out.log 2>&1 &
+  popd
 }
 
 tp_build() {
-  cd "${REPO_DIR}/traffic_portal"
+  pushd "${REPO_DIR}/traffic_portal"
   npm ci
   bower install
   grunt dist
 
   cp "${resources}/config.js" ./conf/
   touch tp.log access.log out.log err.log
-  sudo forever --minUptime 5000 --spinSleepTime 2000 -f -o out.log start server.js &
-  tail -f err.log 2>&1 | color_and_prefix "${red_bg}" "Node Err" &
+  sudo forever --minUptime 5000 --spinSleepTime 2000 -f -o out.log start server.js >out.log 2>&1 &

Review comment:
       Yeah, looking at the docs I'm not sure how `-f -o out.log` would work; would it log stdout from the child process to `out.log` and pip the child's `stderr` through its own `stdout`? Unclear. I'll get rid of `-o out.log`, it seems like that's the best way to ensure all the logs get to where they need to be.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org