-
Notifications
You must be signed in to change notification settings - Fork 149
feat!: emit per-chunk streaming span events on backend spans #1496
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
Changes from 4 commits
81ae879
db57a90
cfca37c
761aaca
dd1bf8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ | |
| import datetime | ||
| import enum | ||
| import logging | ||
| import os | ||
| import threading | ||
| from collections import OrderedDict | ||
| from collections.abc import Callable, Coroutine, Iterable, Mapping | ||
|
|
@@ -49,6 +50,7 @@ | |
|
|
||
| from ..plugins.manager import has_plugins, invoke_hook | ||
| from ..plugins.types import HookType | ||
| from .utils import _parse_bool_env | ||
|
|
||
|
|
||
| class CBlock: | ||
|
|
@@ -751,6 +753,9 @@ class _GenerationState: | |
| chunk_size: Minimum number of chunks to stream at a single time. | ||
| first_chunk_received: Whether the first streamed chunk has arrived | ||
| (gates time-to-first-byte recording). | ||
| processed_chunk_index: Monotonic index of the next streamed chunk to be | ||
| processed, incremented once per chunk folded into the value across the | ||
| repeated `astream()` calls of one generation. | ||
| generate: The task driving generation. Linked to `generate_type`. | ||
| generate_type: Determines which functions can resolve the thunk's value. | ||
| generate_extra: Auxiliary generation task; currently only used by hf. | ||
|
|
@@ -770,6 +775,7 @@ class _GenerationState: | |
| queue: asyncio.Queue = field(default_factory=lambda: asyncio.Queue(maxsize=20)) | ||
| chunk_size: int = 3 | ||
| first_chunk_received: bool = False | ||
| processed_chunk_index: int = 0 | ||
| generate: asyncio.Task[None] | None = None | ||
| generate_type: GenerateType = GenerateType.NONE | ||
| generate_extra: asyncio.Task[Any] | None = None | ||
|
|
@@ -853,6 +859,16 @@ def _record_ttfb(self) -> None: | |
| ).total_seconds() * 1000 | ||
| self._gen.first_chunk_received = True | ||
|
|
||
| async def _emit_event(self, event_name: str, **data: Any) -> None: | ||
| """Fire a `generation_event` hook named `event_name` carrying `data`, if any plugin subscribes.""" | ||
| if has_plugins(HookType.GENERATION_EVENT): | ||
| from ..plugins.hooks.generation import GenerationEventPayload | ||
|
|
||
| event_payload = GenerationEventPayload( | ||
| generation_id=self._call.generation_id, event_name=event_name, data=data | ||
| ) | ||
| await invoke_hook(HookType.GENERATION_EVENT, event_payload) | ||
|
|
||
| async def cancel_generation(self, error: Exception | None = None) -> None: | ||
| """Cancel an in-progress streaming generation, drain the queue, and fire the `generation_error` hook. | ||
|
|
||
|
|
@@ -1153,9 +1169,20 @@ async def astream(self) -> str: | |
|
|
||
| raise chunks[-1] | ||
|
|
||
| emit_chunk_events = self.generation.streaming and _parse_bool_env( | ||
| os.getenv("MELLEA_GENERATION_CHUNK_EVENTS", ""), default=False | ||
| ) | ||
|
Comment on lines
+1172
to
+1174
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a performance hit to doing the os.getenv multiple times here? Should we move that specific check into some code that runs once per import?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can check, this was in a helper originally so I may have messed that up when I moved this inline
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked into this and this is trivial as a performance hit, but I can cache the env var if you're still concerned. It's worth noting this will only run when streaming and only once per |
||
| for chunk in chunks: | ||
| assert self._gen.process is not None | ||
| prev_len = len(str(self._underlying_value or "")) | ||
| await self._gen.process(self, chunk) | ||
| if emit_chunk_events: | ||
| await self._emit_event( | ||
| "chunk_processed", | ||
| chunk_index=self._gen.processed_chunk_index, | ||
| chunk_text_length=len(str(self._underlying_value or "")) - prev_len, | ||
| ) | ||
| self._gen.processed_chunk_index += 1 | ||
|
|
||
| if do_set_computed: | ||
| assert self._underlying_value is not None | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,6 +83,32 @@ class GenerationErrorPayload(MelleaBasePayload): | |
| generation_id: str | None = None | ||
|
|
||
|
|
||
| class GenerationEventPayload(MelleaBasePayload): | ||
| """Payload for `generation_event` — a milestone event during a single generation. | ||
|
|
||
| A generic carrier for events emitted mid-generation. Subscribers branch on | ||
| `event_name` and read the keys `data` carries for that event. | ||
|
|
||
| Known events: | ||
| `chunk_processed`: emitted once per streamed chunk during `astream()` | ||
| (opt-in via `MELLEA_GENERATION_CHUNK_EVENTS`). `data` keys: | ||
| `chunk_index` (int), `chunk_text_length` (int). | ||
|
Comment on lines
+93
to
+95
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel that at least for the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My reason for not making data classes is that these events are not intended to be user consumed like Streaming APOI events are. The hooks are available for devs, but their content is essentially telemetry data. I also didn't want to be making a bunch of new individual event hooks either. If this is an issue I'm ok either making event data classes or separate event hooks, I just felt this wants worth it as I saw this as a dev interface compared to streamings user interface |
||
|
|
||
| Attributes: | ||
| generation_id: Mellea-side hook correlation ID matching the corresponding | ||
| pre_call payload, distinct from the provider-assigned | ||
| `GenerationMetadata.response_id`. `None` when the firing site did not | ||
| generate one. | ||
| event_name: Identifies the event. Subscribers dispatch on this. | ||
| data: Values for this event, keyed by name. The keys present depend on | ||
| `event_name` (see Known events above). | ||
| """ | ||
|
|
||
| generation_id: str | None = None | ||
| event_name: str = "" | ||
| data: dict[str, Any] = {} | ||
|
|
||
|
|
||
| class GenerationBatchPreCallPayload(MelleaBasePayload): | ||
| """Payload for `generation_batch_pre_call` — fires once before a batch generation request. | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.