fix(ollama): pre-imported funcs instrumentation failure

[minimAluminiumalism]

• I have added tests that cover my changes.
• If adding a new instrumentation or changing an existing one, I've added screenshots from some observability platform showing the change.
• PR name follows conventional commits format: feat(instrumentation): ... or fix(instrumentation): ....
• (If applicable) I have updated the documentation accordingly.

Fixes #2866

---

Background
In many scenarios related to ollama, we write code like this which is invalid for instrumentation.

from traceloop.sdk import Traceloop
from ollama import chat

Traceloop.init()

Looking at packages/sample-app/.venv/lib/python3.12/site-packages/ollama/__init__.py, the module exports a chat name which is simply an alias for _client.chat, established via chat = _client.chat. Since this assignment happens during the module's initialization, code using from ollama import chat effectively captures the current reference of _client.chat and binds it to the chat name in its own scope.

_client = Client()

generate = _client.generate
chat = _client.chat
embed = _client.embed

Later modifications(applying wrappers for instrumentation) to the source method (Client.chat) do not propagate to these pre-existing imported references. The imported chat continues to point to the object it referenced at the time of import.

Verification
If we import chat(generate) only when use it rather than import it immediately at the entry of the file, the instrumentation works fine.

from traceloop.sdk import Traceloop
import ollama # do not import chat()

Traceloop.init()
...
# instrumentation works
ollama.chat() 
...

While our unit tests(packages/opentelemetry-instrumentation-ollama/tests/test_chat.py) follow this pattern, I don't think they're sufficient to catch the pre-imported function instrumentation failure.

Solution

for module_name, module in sys.modules.items():
    for attr_name, attr_value in module.__dict__.items():
        if attr_name == "generate" and attr_value.__module__.startswith("ollama"):
            module.__dict__[attr_name] = InstrumentedFunction("generate")

Using the above code we scan every loaded module and find any attribute named chat (or generate, embeddings, etc.) whose implementation lives in the ollama package, and replace it with our dynamic proxy. That proxy then delegates to the latest ollama.generate under the hood—so no matter how the user imported the function, their calls still get instrumented.

---

Important

Fixes instrumentation failure for pre-imported Ollama functions by introducing a proxy class and replacing references in other modules.

• Behavior:
  • Fixes instrumentation failure for pre-imported functions in Ollama by replacing references in other modules.
  • Introduces InstrumentedFunction proxy class to dynamically call latest Ollama function implementations.
• Instrumentation:
  • Modifies _instrument() in __init__.py to iterate through sys.modules and replace references to imported Ollama functions with InstrumentedFunction.
  • Logs replacement actions and handles exceptions with warnings.

^{This description was created by for 883dd00. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to 883dd00 in 2 minutes and 49 seconds. Click for details.

• Reviewed 56 lines of code in 1 files
• Skipped 1 files when reviewing.
• Skipped posting 7 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/__init__.py:7

• Draft comment:
  Consider adding a comment for why sys is imported; this helps future readers understand its purpose.
• Reason this comment was not posted:
  Confidence changes required: 10% <= threshold 50% None

2. packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/__init__.py:435

• Draft comment:
  Remember that this proxy does not preserve attributes (name, doc, etc.) of the wrapped function. Consider using functools.update_wrapper if needed.
• Reason this comment was not posted:
  Marked as duplicate.

3. packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/__init__.py:494

• Draft comment:
  The dynamic replacement loop iterates through all sys.modules. Ensure that replacing attributes in already-imported modules does not have unintended side effects.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

4. packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/__init__.py:508

• Draft comment:
  Rather than simply logging exceptions on instrumentation failure, consider logging the full stack trace to assist in debugging.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50% None

5. packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/__init__.py:7

• Draft comment:
  Importing sys is required for module iteration. This addition is appropriate.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

6. packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/__init__.py:493

• Draft comment:
  The try block that iterates over sys.modules to replace pre-imported functions is effective but may impact performance in apps with many modules. Also, note that it won't update functions already bound in closures. Consider documenting these limitations.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While the comment raises valid technical points, it's not clearly actionable. The performance impact is speculative without evidence of actual problems. The closure limitation is a fundamental limitation of Python that can't really be fixed. The comment suggests documenting these, but documentation suggestions aren't useful PR comments. The performance concern could be valid in large applications. Not documenting these limitations could lead to confusion for users later. However, we should wait for actual performance problems to be reported before optimizing. Documentation requests should go through issues/docs process, not PR comments. Delete the comment. While technically accurate, it's not actionable enough to be a useful PR comment. Documentation requests should be handled separately.

7. packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/__init__.py:251

• Draft comment:
  The async function is named '_aaccumulate_streaming_response', which includes an extra 'a'. Consider renaming it to something like '_accumulate_streaming_response_async' for clarity and consistency with the synchronous version '_accumulate_streaming_response'.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_2voRnD1omUgSh3lU

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[nirga]

meh I'm not a huge fan of that - this can be extremely slow and will stall the app at startup - can you elaborate why this needed / how it solves the issue?

[minimAluminiumalism]

I've added modification details to the PR description.

[nirga]

Thanks. Can't we wrap the client class directly? (And then it should catch that specific instance as well)

[nirga]

@minimAluminiumalism making sure you saw my comment here ⬆️

[nirga]

This logic is going to significantly affect the start time of apps - so I'm super hesitant. Ideally we should be able to instrument some method and then it should be wrapped - no matter when the singleton is created. Have you tried wrapping something else with the "otel-native" method (that we have now) and see if it resolves the issue?

[nirga]

@minimAluminiumalism making sure you saw my comment here ⬆️

[minimAluminiumalism]

This logic is going to significantly affect the start time of apps - so I'm super hesitant. Ideally we should be able to instrument some method and then it should be wrapped - no matter when the singleton is created. Have you tried wrapping something else with the "otel-native" method (that we have now) and see if it resolves the issue?

Thanks for the instruction and I will try that :)

[minimAluminiumalism]

@nirga I've removed the original logic of iterating all loaded modules and wrapping/instrumenting some of them, and change it to instrument func Client._request directly.
I've also modified the unit tests to cover this scenario.