Conversation
fed5726 to
2240219
Compare
2240219 to
3a9e56d
Compare
| # THEN the span are returned bottom to top | ||
| assert read_from_opentelemetry_span(span=spans[1], key=HUMANLOOP_FILE_KEY)["tool"] | ||
| assert read_from_opentelemetry_span(span=spans[2], key=HUMANLOOP_FILE_KEY)["prompt"] | ||
| # assert read_from_opentelemetry_span(span=spans[3], key=HL_FILE_OT_KEY)["flow"] |
There was a problem hiding this comment.
No longer relying on TRACE_FLOW_CONTEXT here, using the ancestor to determine that logs are uploaded correctly.
|
|
||
| # Wait for the Prompt span to be exported; It was asynchronously waiting | ||
| # on the OpenAI call span to finish first | ||
| time.sleep(1) |
There was a problem hiding this comment.
Added some time.sleep in tests since the processor now waits async before sending the HL spans to processor
| assert flow_span_flow_metadata["is_flow_log"] | ||
|
|
||
| # THEN one of the flows is nested inside the other | ||
| spans: list[ReadableSpan] = [mock_export_method.call_args_list[i][0][0][0] for i in range(1, 5)] |
There was a problem hiding this comment.
Order of spans is no longer guaranteed due to async mechanism in processor, tests should no longer rely or test for order
c3cc96b to
11ff653
Compare
| self._upload_queue.task_done() | ||
|
|
||
| def _mark_span_completed(self, span_id: int) -> None: | ||
| for flow_log_span_id, flow_children_span_ids in self._flow_log_prerequisites.items(): |
There was a problem hiding this comment.
potentially dumb question: Right now we mark flow as complete only if self._flow_log_prerequisites.items() is not empty - is it always the case? Aka do we ever need to mark it as complete when self._flow_log_prerequisites.items() is empty
src/humanloop/otel/exporter.py
Outdated
| span.name, | ||
| parent_span_id, | ||
| ) | ||
| time.sleep(0.1) |
There was a problem hiding this comment.
discussed offline - if time.sleep potentailly increases latency - maybe remove it
| provider="openai", | ||
| max_tokens=-1, | ||
| temperature=0.7, | ||
| <<<<<<< HEAD |
| @@ -1,3 +1,6 @@ | |||
| [project] | |||
| name = "humanloop" | |||
There was a problem hiding this comment.
Is it necessary, aka why was it added now?
olehvell-h
left a comment
There was a problem hiding this comment.
make sense to me!
I haven't propertly understood all logic, so don't count to much on my review. At the same time I don't see a reason not to approve it. Maybe spend a bit of extra time on qaing just to make sure.
humanloop.promptorhumanloop.flow. This is also more dogmatic with how OT does thingsThings to look out for: I am introducing some async wait logic for the streaming case in the processor. I use a thread pool to not block the main thread. Please double-check if it makes sense and if there are no gotchas