Skip to content

Conversation

@Hansehart
Copy link
Contributor

Related Issues

Proposed Changes:

Adopting https://langfuse.com/docs/observability/features/observation-types for better observability. Langfuse types has been adopted where it makes sense. Beside better visual interaction, usage and cost tracking is for the appropriate types improved

How did you test it?

unit

Notes for the reviewer

Added embedder, generator and retriever. Also new logic for checking for generator type.

Checklist

@sjrl sjrl requested review from vblagoje and removed request for sjrl November 10, 2025 06:52
@sjrl
Copy link
Contributor

sjrl commented Nov 10, 2025

hey @vblagoje I'm assigning you since you have more context

Copy link
Member

@vblagoje vblagoje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just make this one minor change if you agree. Lowers cognitive load quite a lot.

elif context.operation_name == "haystack.agent.run":
return LangfuseSpan(self.tracer.start_as_current_observation(name=context.name, as_type="agent"))
elif context.component_type in _ALL_SUPPORTED_GENERATORS:
elif context.component_type and context.component_type.endswith("Retriever"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I started this smell here, so let's please update it to something that's easier to read:

span_type = "span"

if context.component_type == "ToolInvoker":
    span_type = "tool"
elif context.operation_name == "haystack.agent.run":
    span_type = "agent"
elif context.component_type and context.component_type.endswith("Retriever"):
    span_type = "retriever"
elif context.component_type and context.component_type.endswith("Embedder"):
    span_type = "embedding"
elif context.component_type and context.component_type.endswith("Generator"):
    span_type = "generation"
return LangfuseSpan(self.tracer.start_as_current_observation(name=context.name, as_type=span_type))

LMK wdyt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right! Much better now.

@vblagoje vblagoje self-assigned this Nov 10, 2025
@vblagoje vblagoje self-requested a review November 11, 2025 09:05
Copy link
Member

@vblagoje vblagoje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok looks gtg, thanks for this contrib @Hansehart, keep them coming

@vblagoje vblagoje merged commit a901b0e into deepset-ai:main Nov 11, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration:langfuse type:documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants