-
Notifications
You must be signed in to change notification settings - Fork 3
SED-4409 Keyword calls with no agent response have no measurements #561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SED-4409 Keyword calls with no agent response have no measurements #561
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
jeromecomte
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requiring some clarification.
If possible we should also cover with JUnit
...ans/step-plans-base-artefacts/src/main/java/step/artefacts/handlers/CallFunctionHandler.java
Outdated
Show resolved
Hide resolved
...ans/step-plans-base-artefacts/src/main/java/step/artefacts/handlers/CallFunctionHandler.java
Outdated
Show resolved
Hide resolved
...ans/step-plans-base-artefacts/src/main/java/step/artefacts/handlers/CallFunctionHandler.java
Show resolved
Hide resolved
| Map<String, Object> data = new HashMap<>(); | ||
| data.put(MeasureTypes.ATTRIBUTE_TYPE, MeasureTypes.TYPE_KEYWORD); | ||
| List<Measure> measures = Objects.requireNonNullElse(node.getMeasures(), new ArrayList<>()); | ||
| measureName = Objects.requireNonNullElse(measureName, node.getName()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about this default. If we cannot resolve the KW name, this means, that the function couldn't be found. Do we really create this measure in this case? this default works only if hte nodeName is equals to the KW name, which is also not a guaranty. Instead, we could use a specific name: "Unresoved" or something specific. Not sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really sure either. But if we really want to not miss any keyword call errors in the measurements / response time time-series, then I guess we should also create a measure in this case. As for the fall back name then I would propose to use the node name (as it gives a hint) + "_Unresolved". Example for a KW added from the component list and a CallKeyword by attributes:

...ans/step-plans-base-artefacts/src/main/java/step/artefacts/handlers/CallFunctionHandler.java
Outdated
Show resolved
Hide resolved
...ans/step-plans-base-artefacts/src/main/java/step/artefacts/handlers/CallFunctionHandler.java
Show resolved
Hide resolved
|
Agree with your proposition.
…On Fri, Nov 28, 2025, 14:31 David Stephan ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
step-plans/step-plans-base-artefacts/src/main/java/step/artefacts/handlers/CallFunctionHandler.java
<#561 (comment)>:
> +
+ /**
+ * A Call function shall always have a measure of type "keyword", if the output measure is empty (mostly happen in case the agent call was not completed due to interruption), we create one directly here
+ * Note don't use Map.of(), List.of() as these could be enriched with more content later
+ * The status is left null to keep the default logic: node status is used when converting Measure to Measurement by the MeasurementPlugin
+ * @param node the node for which we ensure a keyword measure exists
+ * @param measureName the name of the measure to be created, fallback to node name if null
+ * @param endTime the measure endtime, fallback to current time if null
+ * @param startTime the startTime of the measure
+ */
+ private static void createKeywordMeasureIfAbsent(CallFunctionReportNode node, String measureName, Long endTime, long startTime) {
+ if (node.getMeasures() == null || node.getMeasures().isEmpty()) {
+ Map<String, Object> data = new HashMap<>();
+ data.put(MeasureTypes.ATTRIBUTE_TYPE, MeasureTypes.TYPE_KEYWORD);
+ List<Measure> measures = Objects.requireNonNullElse(node.getMeasures(), new ArrayList<>());
+ measureName = Objects.requireNonNullElse(measureName, node.getName());
I'm not really sure either. But if we really want to not miss any keyword
call errors in the measurements / response time time-series, then I guess
we should also create a measure in this case. As for the fall back name
then I would propose to use the node name (as it gives a hint) +
"_Unresolved". Example for a KW added from the component list and a
CallKeyword by attributes:
image.png (view on web)
<https://github.com/user-attachments/assets/1cef0d42-7f57-4cc8-9b4c-1db8e51b2b87>
—
Reply to this email directly, view it on GitHub
<#561 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEG24ZGVSCYFIKY575C23PD37BFDJAVCNFSM6AAAAACNOO7P2WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTKMJZGAYTSNJUGY>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
#579) * SED-4350 extend automation package test scope (#558) * SED-4305: unit tests for AutomationPackageManager * SED-4305: forbid libraries in local execution (CLI) * SED-4305: fix test * SED-4305: new unit test * SED-4305: remove redundant todo * SED-4350: new unit test * SED-4350: merge master into SED-4350 * SED-4350: fix test * SED-4350: unit test * SED-4350: unit test (cherry picked from commit 6be0173) * SED-4350 adapting Junit after merge * SED-4409 Keyword calls with no agent response have no measurements (#561) * SED-4409 Keyword calls with no agent response have no measurements * SED-4409 PR feedback * SED-4409 PR feedback + Junit * SED-000 AP Junit test flakiness * SED-4403 deploy library error messages are not clear enough (#562) SED-4404: improve error messages (cherry picked from commit 011fbcb) * SED-4428 Uploading an AP defining K6 keywords without scriptDirectory fails (#565) * SED-4413 added connect and read timeouts to AbstractRemoteClient (#563) * SED-4413 added connect and read timeouts to AbstractRemoteClient * SED-4413 PR feedback (cherry picked from commit 984f98e) * SED-4401 Application context of step-functions-handler-initializer.jar not released (#567) * SED-4401 Application context of step-functions-handler-initializer.jar not released * SED-4413 added connect and read timeouts to AbstractRemoteClient (#563) * SED-4413 added connect and read timeouts to AbstractRemoteClient * SED-4413 PR feedback (cherry picked from commit 984f98e) * SED-4401 Application context of step-functions-handler-initializer.jar not released * Revert "SED-4413 added connect and read timeouts to AbstractRemoteClient (#563)" This reverts commit 5eff1e9. * SED-4401 Fixing NoClassDefFound * SED-4401 Fixing NPE --------- Co-authored-by: Jonathan Rubiero <30461894+rubij@users.noreply.github.com> * SED-4413 Re-added connect and read timeouts to AbstractRemoteClient after merge of SED-4401 (#563) * SED-4439 Optimize reporting request and query performance for Step 29… (#569) SED-4439 Optimize reporting request and query performance for Step 29 branch * SED-4387 from a project it is possible to use bulk deletion to delete resources of a global project (#568) * SED-4387 delete resource services do not check write access for tenant * SED-4387 improve error message displyed in the UI * SED-4444 timeseries-re-ingestion-doesnt-work-for-psql (#570) * SED-4444 timeseries-re-ingestion-doesnt-work-for-psql * SED-4444 timeseries-re-ingestion-doesnt-work-for-psql * SED-4443 ClassCastException in FunctionMessageHandler (#572) * SED-4443 Fixing ClassCastException in FunctionMessageHandler * SED-4443 Fixing ClassCastException in FunctionMessageHandler * SED-4443 Switching to explicit creation of initializer CL (#574) * SED-4445 bumping framework to 2.5.1 * SED-4418 Calling Keywords in an after section does not release tokens (#575) * SED-4418 Calling Keywords in an after section does not release tokens * SED-4418 Calling Keywords in an after section does not release tokens * SED-4418 Adding a junit test * SED-4287 ForEach may try to use uninitialized tempWriter (#576) * SED-4287 ForEach may try to use uninitialized tempWriter * SED-4287 propagating exception * SED-4340 find-usages-of-keyword-and-plan-referenced-by-attribute-is-broken * SED-4340 PR feedbacks * SED-4340 PR feedbacks * SED-4340 fixing incorrect context and predicate usage * SED-4471 Add audit logging to parameters, schedules, resources (#581) * SED-4471 Add audit logging to parameters and schedules * SED-4471 Code simplification * SED-4471 More logging * EI-485 new nexus staging host * SED-4430 Improve Agent and CLI resilience in case of network errors (#580) * SED-4430 Improve Agent and CLI resilience in case of network errors * SED-4430 Reverting method signatures and correct comment * EI-485 new nexus staging host * SED-4430 bumping grid and FW after PR merge --------- Co-authored-by: Jonathan Rubiero <30461894+rubij@users.noreply.github.com> * SED-4480 BE Junit test flakiness * SED-4498 Parameters priority not respected when deployed with AP (#584) * SED-4498 Parameters priority not respected when deployed with AP * SED-4498 PR feedbacks * SED-4498 fixing junit * SED-4506 Clean-up of isolated AP resources delete all revisions and f… (#585) SED-4506 Clean-up of isolated AP resources delete all revisions and files but keep the resource entry in DB * SED-4451 the performance of the analytics view is very poor on large datasets after re ingestion (#578) * SED-4451 The performance of the analytics view is very poor on large datasets after re-ingestion * SED-4451 The performance of the analytics view is very poor on large datasets after re-ingestion * SED-4451 The performance of the analytics view is very poor on large datasets after re-ingestion * SED-4451 adapting TimeSeriesExecutionPlugin * SED-4451 index optimizations and other perf fixes * SED-4451 bumping framework * SED-4451 PR Feedbacks * SED-4451 adding missing TS index and fixing default resolution * SED-4451 Bumping framework to 0.0.0-25-SNAPSHOT before merge * SED-4340 fixing logging * SED-4340 fixing error handling * SED-4430 improving warning message --------- Co-authored-by: Igor Egorov <118996755+iegorov777@users.noreply.github.com> Co-authored-by: Jérôme Comte <jerome.comte@exense.ch> Co-authored-by: Jonathan Rubiero <30461894+rubij@users.noreply.github.com> Co-authored-by: Christoph Langguth <christoph.langguth@exense.ch>
No description provided.