joeyutong commented on code in PR #860:
URL: https://github.com/apache/flink-agents/pull/860#discussion_r3535895671
##########
runtime/src/main/java/org/apache/flink/agents/runtime/python/utils/PythonResourceAdapterImpl.java:
##########
@@ -200,6 +203,11 @@ public Object callMethod(Object obj, String methodName,
Map<String, Object> kwar
return interpreter.invoke(CALL_METHOD, obj, methodName, kwargs);
}
+ @Override
+ public void setMetricGroup(Object pythonResource, FlinkAgentsMetricGroup
metricGroup) {
+ interpreter.invoke(SET_METRIC_GROUP, pythonResource, metricGroup);
Review Comment:
@weiqingy One ownership detail I think is still worth calling out: this is
not only about avoiding a second recorder after forwarding the metric group. It
is also about which effective metric group the action-level recorder should use
after the resource call returns.
For example, in the Java action -> Python chat model path, the Python
resource may wrap the forwarded action metric group with a provider-specific
dimension such as `modelProvider`. Metrics recorded inside the Python resource
would use that wrapped group, but token metrics are still recorded later by the
Java action/model-setup path, which only sees the Java wrapper's metric group.
That means action-recorded token metrics may still miss the provider-specific
dimension.
So I think the ownership question also needs to define whether a
resource-derived metric scope should flow back to the action-level recorder.
This PR fixes forwarding into the resource, but not that return path.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
