Skip to content
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

Clement/eng 2181 otel exporter #157

Merged
merged 10 commits into from
Jan 9, 2025
Merged

Conversation

clementsirieix
Copy link
Contributor

@clementsirieix clementsirieix commented Dec 16, 2024

Changes

  • Deprecate instrumentations call
  • Added initialize method to the client
  • Added a set_properties method for customization
  • Added a custom Traceloop exporter to handle llm spans

How to test?

Test deprecations

  • Test any examples that use an explicit instrumentation, notice the deprecation log

Tests

  • The basic streaming test LITERAL_API_URL="http://localhost:3000" LITERAL_API_KEY="my-initial-api-key" python3 examples/streaming.py
Screenshot 2024-12-16 at 17 45 55
  • The langchain variables example LITERAL_API_URL="http://localhost:3000" LITERAL_API_KEY="my-initial-api-key" python3 examples/langchain_variable.py
Screenshot 2024-12-16 at 17 46 13
  • Test multimodal query LITERAL_API_URL="http://localhost:3000" LITERAL_API_KEY="my-initial-api-key" python3 examples/multimodal.py
Screenshot 2024-12-16 at 17 45 30
  • Test llamaindex flow LITERAL_API_URL="http://localhost:3000" LITERAL_API_KEY="my-initial-api-key" python3 examples/llamaindex_workflow.py

Screenshot 2024-12-17 at 17 41 54

@clementsirieix clementsirieix added the enhancement New feature or request label Dec 16, 2024
@clementsirieix clementsirieix self-assigned this Dec 16, 2024
@clementsirieix clementsirieix force-pushed the clement/eng-2181-otel-exporter branch from 51602d3 to 95cdb03 Compare December 16, 2024 16:54
@@ -119,6 +128,13 @@ def instrument_llamaindex(self):

instrument_llamaindex(self.to_sync())

def initialize(self):
with redirect_stdout(io.StringIO()):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Traceloop is verbose so this is aimed to prevent explicit print statement.

root_run = active_root_run_var.get()
parent = active_steps_var.get()[-1] if active_steps_var.get() else None

Traceloop.set_association_properties(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not an upsert so we need to maintain explicit calls.


Traceloop.set_association_properties(
{
"literal.thread_id": str(thread.id) if thread else "None",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can only store strings

Comment on lines 59 to 66
# # TODO: Add generation promptid
# # TODO: Add generation variables
# # TODO: Check missing variables
# # TODO: ttFirstToken
# # TODO: duration
# # TODO: tokenThroughputInSeconds
# # TODO: Add tools
# # TODO: error check with gemini error
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haven't found a proper way to handle those as of now:

  • promptid and variables could be set manually?
  • missing variables like frequency_penalty are just not tracked by Traceloop so they do not appear in the span attributes, could only found basic support in their issue.
  • ttFirstToken, duration and tokenThroughputInSeconds could not find a clear alternative
  • tools to test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could not find a way to handle gemini and ttFirstToken

@clementsirieix clementsirieix force-pushed the clement/eng-2181-otel-exporter branch from deca886 to e09d261 Compare December 17, 2024 12:55
@clementsirieix clementsirieix force-pushed the clement/eng-2181-otel-exporter branch from 327ffa3 to 1cdace9 Compare December 17, 2024 15:26
@@ -549,6 +566,21 @@ def __enter__(self) -> Step:

if active_root_run_var.get() is None and self.step_type == "run":
active_root_run_var.set(self.step)
Traceloop.set_association_properties(
Copy link
Contributor

Choose a reason for hiding this comment

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

if otel is not initialized this wont be an issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it can be safely called even without initialization 👍

@clementsirieix
Copy link
Contributor Author

As discussed yesterday:

@willydouhard willydouhard merged commit 843de0c into main Jan 9, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants