New logs#10
Conversation
LFRequest gains an optional session_id_ that is sent as the same X-LFJson-Session header lanforge_api already uses, so entries in our lightweight api log can be joined against the GUI's own request logs instead of only being locally-recorded labels. LFCliBase/Realm accept an optional _lf_session (an lanforge_api.LFSession) and stamp its session id on every request and log line. lf_webpage.py mints a real LFSession when --save_api is passed, as a reference implementation. Also makes LFRequest.default_headers an instance-level copy instead of mutating the shared class-level dict, so one script's session id can't leak into another LFRequest instance's headers. Signed-off-by: Sidartha-CT <neelapu.sidartha@candelatech.com>
_log_api_call now writes through a cached logging.handlers.RotatingFileHandler (default 10MB x 10 backups, both overridable) instead of raw file writes with a truncate-on-construct step. This keeps the log safe to leave enabled indefinitely (e.g. save_api on by default for diagnosing customer systems) without either growing unbounded or losing history on every run. A module-level logger cache keyed by absolute path avoids stacking duplicate handlers if multiple LFCliBase/Realm instances point at the same file. Since the log file is now shared and persistent across runs rather than scoped to one run, lf_webpage.py's report-folder copy switches from a plain file copy to _copy_api_log_for_session(), which filters entries down to the current run's session id (falling back to copying everything if no session was established). Signed-off-by: Sidartha-CT <neelapu.sidartha@candelatech.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93ce1eea8d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # mint a LANforge GUI session id so our lightweight api log can be correlated | ||
| # with the GUI's own request logs (same X-LFJson-Session cookie on both) | ||
| lanforge_api = importlib.import_module("lanforge_client.lanforge_api") | ||
| lf_session = lanforge_api.LFSession(lfclient_url="http://%s:%s" % (args.mgr, args.mgr_port), |
There was a problem hiding this comment.
When --save_api is used with multiple bands (including the default band list), this loop creates a new LFSession for each band. Earlier bands' API log entries are tagged with earlier session ids, but after the loop the report is generated from only the last http object and _copy_api_log_for_session(..., self.local_realm._session_id()) filters for that one id, so the report omits the API calls from all previous bands. Use one session for the whole run or copy entries for every session created in the loop.
Useful? React with 👍 / 👎.
| _copy_api_log_for_session(self.local_realm.api_log_filename, report_path_date_time, | ||
| self.local_realm._session_id()) |
There was a problem hiding this comment.
When --save_api is enabled on a long run that writes more than the 10 MB default log size, the rotating handler moves earlier entries into lf_api_calls.log.1, .2, etc., but this report copy reads only the active lf_api_calls.log. The generated report therefore drops the older API calls from the same run once rotation occurs; scan/copy the rotated backup files as well when filtering by session.
Useful? React with 👍 / 👎.
| self.local_realm = realm.Realm(lfclient_host=self.host, lfclient_port=self.port, | ||
| _save_api=_save_api, _api_log_file_name=_api_log_file_name) | ||
| _save_api=_save_api, _api_log_file_name=_api_log_file_name, | ||
| _lf_session=_lf_session) |
There was a problem hiding this comment.
With --save_api even for a single band, this only gives the session/log settings to local_realm; the HTTPProfile created immediately after still initializes its own LFCliBase with defaults, and its start_cx(), stop_cx(), and cleanup() methods call self.json_post(...). Those traffic-control REST calls are therefore sent without X-LFJson-Session and are absent from the saved API log, so the correlated report misses the requests that actually start and stop the test.
Useful? React with 👍 / 👎.
Extracts the rotating-logger cache and log-line formatting out of lfcli_base.py into a standalone lanforge_client module, so the same logging can eventually be reused by lanforge_api.py's own request layer too, instead of only being available to scripts going through LFCliBase/Realm/LFRequest.py. Signed-off-by: Sidartha-CT <neelapu.sidartha@candelatech.com>
BaseLFJsonRequest (get/get_as_json/json_post, and json_put/json_delete/ json_get via delegation) now calls the same lanforge_client.api_call_logger module LFRequest.py uses, so scripts that talk to the GUI directly through lanforge_api.py (raw_cli.py, lf_add_profile.py, etc.) get identical save_api logging instead of only scripts going through LFCliBase/Realm. BaseSession/LFSession gain save_api/api_log_file_name/api_log_max_bytes/ api_log_backup_count, mirroring LFCliBase's constructor. print_diagnostics now returns a one-line summary (reason + X-Error-* headers) that gets captured as last_diagnostics, same pattern used in LFRequest.py. Logging is emitted once per outer json_get/json_post/json_put/json_delete call rather than per internal retry attempt, matching the granularity already used on the LFRequest.py side. Signed-off-by: Sidartha-CT <neelapu.sidartha@candelatech.com>
|
@codex review. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98e2a18ac8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if session_id is None: | ||
| shutil.copy(src_path, dest_path) | ||
| return | ||
| with open(src_path) as src, open(dest_path, 'w') as out: |
There was a problem hiding this comment.
When a --save_api run exceeds the rotating logger's maxBytes, earlier entries for the same session are moved to files like <log>.1, but this copy path filters only the active src_path. In long or API-heavy tests the generated report will silently omit the beginning of the run, which makes the saved diagnostics incomplete; include the rotated backups when copying/filtering the session log.
Useful? React with 👍 / 👎.
| responses_=responses, | ||
| error_=herror, | ||
| debug_=debug, | ||
| die_on_error_=die_on_error) |
There was a problem hiding this comment.
When lanforge_api hits a fatal error path, such as a 400-class response that sets die_on_error = True or a caller that requested die_on_error, this call passes that flag into print_diagnostics; that helper calls exit(1) before returning in this mode, so the _log_api_call immediately below is never reached. With save_api enabled, the failures that abort the script are absent from the API log; log before invoking an exiting helper or call it without exiting and exit after logging.
Useful? React with 👍 / 👎.
| if responses[0] is None: | ||
| if debug: | ||
| self.logger.debug(msg="No response from " + url) | ||
| self._log_api_call(method_, url, response_code=self.last_response_code, |
There was a problem hiding this comment.
Because last_response_code and last_diagnostics are instance fields and are not cleared at the start of each request, a reused LFJsonQuery can carry a previous response code into a later call that gets no response, such as a URLError. This log entry then writes the stale code/status for the new failure, so the API log can show a failed call as response_code: 200/OK; reset these fields before each request or set the code to None on no-response paths.
Useful? React with 👍 / 👎.
| if error is not None: | ||
| status = "ERROR" | ||
| elif response_code is not None: | ||
| status = "OK" if 200 <= response_code < 300 else "ERROR" |
There was a problem hiding this comment.
The request layer explicitly treats several non-2xx responses as acceptable (BaseLFJsonRequest.OK_STATUSES includes redirects and 404 for normal polling/missing-resource cases), but this logger marks anything outside 2xx as ERROR. In workflows that expect temporary 404s while waiting for resources, the saved API log/report will report normal calls as failures and obscure real errors; align this status calculation with the request layer's accepted status set.
Useful? React with 👍 / 👎.
| # mint a LANforge GUI session id so our lightweight api log can be correlated | ||
| # with the GUI's own request logs (same X-LFJson-Session cookie on both) | ||
| lanforge_api = importlib.import_module("lanforge_client.lanforge_api") | ||
| lf_session = lanforge_api.LFSession(lfclient_url="http://%s:%s" % (args.mgr, args.mgr_port), |
There was a problem hiding this comment.
Because this creates a fresh session inside the for bands in args.bands loop, a multi-band --save_api run records earlier bands under different session ids, but report generation happens once after the loop and copies only http.local_realm._session_id() from the last http instance. The generated report therefore drops all API calls from previous bands; create one session before the band loop or aggregate all per-band session ids when filtering the log.
Useful? React with 👍 / 👎.
1 participant
No description provided.