-
Notifications
You must be signed in to change notification settings - Fork 742
feat(ollama): implement emitting events in addition to current behavior #2891
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
Open
LuizDMM
wants to merge
9
commits into
traceloop:main
Choose a base branch
from
LuizDMM:implement-events-ollama
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… current behavior * Add "use_legacy_attributes" to Config and the Instrumentor constructor, defaulting to True; * emit events for user prompts and AI responses, following [OpenTelemetry semantic conventions](https://opentelemetry.io/docs/specs/semconv/gen-ai/gen-ai-events/\\\); * introduce a privacy safeguard by checking the OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT environment variable to enable or disable content capture in events; * implement comprehensive tests to verify the new functionality and ensure that, when use_legacy_attributes == True, the existing behavior remains unchanged;
- Introduce TypedDicts and dataclasses to standardize inputs for event handling; - Move `event_logger` to Config to centralize configuration and reduce complexity; - Add `emit_event` method to encapsulate all event emission logic via `Event` instances; - Refactor instrumentation to use `emit_event`, improving clarity and maintainability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Changes requested ❌
Reviewed everything up to 8de90b2 in 4 minutes and 16 seconds. Click for details.
- Reviewed
4401
lines of code in38
files - Skipped
0
files when reviewing. - Skipped posting
33
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/tests/test_generation.py:12
- Draft comment:
Verify that embedding responses convert to a proper dict for the event payload. Consider explicitly checking the type of response['embedding'] to avoid surprises. - Reason this comment was not posted:
Comment was on unchanged code.
2. packages/opentelemetry-instrumentation-ollama/tests/conftest.py:101
- Draft comment:
The fixture updates an environment variable; ensure that tests running concurrently won't get side effects. It might be safer to explicitly backup and restore the env var. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The fixture is already using function scope which means it runs separately for each test. The cleanup is handled properly in the yield fixture teardown which runs even if tests fail. The env var is properly removed after each test. Pytest handles test isolation well by default. The current implementation follows standard pytest patterns. The comment raises a valid concern about test isolation. Environment variables are global state that could theoretically cause issues. The fixture's function scope and proper cleanup via yield already handles this correctly following pytest best practices. The suggested "backup and restore" approach wouldn't provide meaningful additional safety. Delete the comment. The current implementation already follows best practices for test isolation and env var management in pytest fixtures.
3. packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/event_handler.py:100
- Draft comment:
When removing keys from the event dict (e.g. 'tool_calls' in _emit_message_event), ensure that this does not inadvertently lose information. A comment documenting why removal is safe is good. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to ensure that removing keys from a dictionary does not lose information and to document why it's safe. This falls under asking the author to ensure behavior is intended and to document it, which violates the rules.
4. packages/opentelemetry-instrumentation-ollama/tests/test_chat.py:87
- Draft comment:
The tests comparing log event bodies use 'assert dict(...) == expected_content'. Consider using deep comparison methods to allow additional unspecified keys. - Reason this comment was not posted:
Confidence changes required:50%
<= threshold50%
None
5. packages/opentelemetry-instrumentation-ollama/tests/test_generation.py:396
- Draft comment:
Ensure that async generation tests correctly await streaming responses; double-check that token usage assertions are consistent, even though prompt token usage isn’t reported in async mode. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to ensure that async generation tests correctly await streaming responses and to double-check token usage assertions. This violates the rule against asking the PR author to double-check things or ensure behavior is intended. The comment does not provide a specific code suggestion or ask for a specific test to be written.
6. packages/opentelemetry-instrumentation-ollama/tests/conftest.py:100
- Draft comment:
Consider preserving any pre-existing value of OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT rather than simply overwriting it, and restore it after the test to avoid side effects. - 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% This is a test fixture that's function-scoped, meaning it runs fresh for each test. The code already cleans up by removing the env var after the test. While preserving previous values would be slightly more robust, the current approach is standard practice for test fixtures. The scope is limited to individual test functions, so side effects are contained. The comment raises a valid point about test isolation. There could be edge cases where other tests rely on this env var being set to a specific value. The function scope and cleanup via pop() already provide good test isolation. The suggested change would add complexity for minimal benefit in a testing context. The comment should be deleted. The current implementation is a standard and acceptable pattern for test fixtures, with proper cleanup already in place.
7. packages/opentelemetry-instrumentation-ollama/tests/conftest.py:116
- Draft comment:
Similar feedback applies to the instrument_with_no_content fixture; consider saving and restoring any pre-existing environment value to avoid unexpected test interactions. - 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% This is a legitimate testing best practice - modifying environment variables without preserving their state could cause test interference. However, this is a test-specific environment variable that's unlikely to be set elsewhere. The cleanup is already happening via pop(). The risk of test interference is very low. The comment identifies a real testing best practice. Not preserving env vars could theoretically cause flaky tests or interference between test runs. While technically correct, this seems overly cautious given this is a test-specific env var that's already being cleaned up. The current implementation is sufficient for this specific case. The comment should be removed as it suggests additional complexity that isn't really necessary given the specific context and existing cleanup.
8. packages/opentelemetry-instrumentation-ollama/tests/test_chat.py:96
- Draft comment:
The assert_message_in_logs helper function is duplicated across test files. Consider moving it to a common shared test helper module to avoid duplication. - 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% 1. The comment claims duplication but doesn't provide evidence of where else this function exists 2. Without seeing the other test files, I can't verify if duplication actually exists 3. Even if duplication exists, I need strong evidence to keep a comment suggesting a refactor 4. The comment is about code organization rather than a bug or serious issue 5. Moving test helper functions is a minor optimization that could be handled later I don't have enough context to verify if the function is actually duplicated. The comment could be correct, but I need to see the other test files to confirm. Given the lack of evidence and the relatively minor nature of the suggested change, it's better to err on the side of removing the comment. Test helper functions often need to be customized per test file anyway. Delete the comment. While the suggestion might be valid, I don't have strong evidence of the claimed duplication, and reorganizing test helper functions is a minor optimization that doesn't require immediate attention.
9. packages/opentelemetry-instrumentation-ollama/tests/test_generation.py:37
- Draft comment:
The tests use a hardcoded token count (17) for LLM_USAGE_PROMPT_TOKENS. It may be beneficial to define this as a constant or document its origin to avoid brittle tests. - 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.
10. packages/opentelemetry-instrumentation-ollama/tests/test_chat.py:587
- Draft comment:
Several async tests have commented out assertions regarding prompt token usage. Consider adding a TODO note to track or fix the inconsistency in async token reporting. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The comment identifies a real inconsistency in the code's behavior between sync and async implementations. The suggestion to add a TODO is actionable and would help track this issue. However, the comment is purely informative and doesn't require any immediate code changes. According to the rules, we should not make purely informative comments. The comment correctly identifies a real issue in the codebase. However, if this is an important issue to track, it should be filed as a GitHub issue rather than left as a PR comment. While the comment suggests an actionable step (adding a TODO), according to the rules we should not make purely informative comments or ask authors to add documentation. The comment should be deleted. If this inconsistency is important to track, it should be filed as a GitHub issue rather than discussed in PR comments.
11. packages/opentelemetry-instrumentation-ollama/tests/test_generation.py:11
- Draft comment:
The test functions compare response['response'] directly. It might be prudent to verify the key exists in the response before using it, in case of unexpected schema changes. - Reason this comment was not posted:
Comment was on unchanged code.
12. packages/opentelemetry-instrumentation-ollama/tests/test_embeddings.py:120
- Draft comment:
The repetition of similar assertions across tests (for embeddings, chat, and generation) suggests an opportunity to parameterize or refactor common assertions into reusable helpers. - Reason this comment was not posted:
Comment was on unchanged code.
13. packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/__init__.py:375
- Draft comment:
Typographical error: Change 'unknow llm_request_type' to 'unknown llm_request_type' in the error message of _emit_choice_events. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While this is technically correct - there is a typo - fixing error messages that users will rarely see is a very low priority change. Error messages are not part of the public API and this typo doesn't impact functionality. Additionally, the same typo appears in another error message on line 338 which this comment doesn't address, suggesting an incomplete fix. The typo could theoretically confuse users who see the error message. And consistency in error messages is generally good practice. The benefit is extremely minor compared to the overhead of reviewing and applying the change. Error messages are rarely seen and this typo doesn't impact understanding. Delete this comment as it addresses a trivial cosmetic issue in an error message that doesn't impact functionality.
14. packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/event_handler.py:95
- Draft comment:
Typo: The phrase "it is the same as the in the event name" contains an extra "the". Consider revising it to "it is the same as in the event name" for better clarity. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the comment is technically correct about the grammatical error, it's a very minor issue in a comment that doesn't impact code functionality or readability significantly. The meaning is still clear despite the typo. According to the rules, we should not make comments that are obvious or unimportant. A minor grammatical fix in a comment falls into this category. The typo could potentially confuse readers of the code documentation. Clear documentation is important for maintainability. While clear documentation is important, this particular typo is minor enough that most readers would understand the meaning without issue. The cost of the PR review interaction outweighs the small benefit of fixing this typo. Delete this comment as it points out a very minor grammatical issue that doesn't significantly impact code or documentation quality.
15. packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/utils.py:28
- Draft comment:
Typographical error: 'OpenLLMetry' appears to be a typo. Consider changing it to 'OpenTelemetry' for consistency. - 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.
16. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_chat/test_ollama_async_streaming_chat_legacy.yaml:27
- Draft comment:
Typo: The content field shows "''s". If this is intended to represent an apostrophe (as in "Here's"), consider changing it to a single apostrophe or adjust accordingly for clarity. - 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.
17. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_chat/test_ollama_async_streaming_chat_legacy.yaml:188
- Draft comment:
Typo: There is a stray single quote on line 188 that appears out of place in the JSON response. Please verify if this is intentional or if it should be removed. - 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.
18. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_chat/test_ollama_async_streaming_chat_with_events_with_no_content.yaml:27
- Draft comment:
Typo alert: The response content at line 27 contains "''s" which might be a typographical error. It seems like it could be an unintended double apostrophe (perhaps intended to be "it's"). Please verify if this is correct. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a test fixture file recording actual API responses. The double apostrophe is part of the real response data from the API, not a typo. The comment suggests changing test data that is accurately recording real behavior. We should never modify recorded test responses to "fix" them - that would defeat the purpose of the test cassettes. Could the double apostrophe indicate a bug in the underlying API that should be fixed? Even if it is a bug in the API, this test cassette's job is to accurately record the API's behavior, not to fix it. API fixes should happen in the API code, not in test recordings. Delete this comment. The double apostrophe is part of the actual API response being recorded in this test cassette. Modifying it would make the test data inaccurate.
19. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_chat/test_ollama_async_streaming_chat_with_events_with_no_content.yaml:80
- Draft comment:
Typo alert: The response message content at line 80 consists solely of an isolated double quote ("
). This seems unusual and may be an accidental stray character. Consider checking if this is intentional. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a test cassette file that records API responses. The quote is part of streaming response where each token comes separately. Looking at the surrounding context, it's clear this is intentionally capturing how the LLM broke up the word "instrumented" into pieces including the surrounding quotes. This is expected behavior, not a typo. Could there be some other reason why having an isolated quote character would be problematic, even if it's intentional? No - in streaming responses it's completely normal and expected for tokens to be split this way, including punctuation characters. The test is correctly capturing the actual API behavior. The comment should be deleted because the quote character is not a typo - it's an intentional and correct recording of how the streaming API response was tokenized.
20. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_chat/test_ollama_chat_legacy.yaml:28
- Draft comment:
Typographical error: it appears as "it''s" on line 28. Consider changing it to "it's". - 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.
21. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_chat/test_ollama_streaming_chat_legacy.yaml:109
- Draft comment:
There appears to be a potential typo on line 109 where the response message content is "''s". Consider verifying if it should be changed to "it's" for correct contraction spelling. - 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.
22. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_chat/test_ollama_streaming_chat_with_events_with_content.yaml:108
- Draft comment:
Typographical error: At line 108, the token ''s appears to be a mistake. It likely should be "it's". Please review if this is the intended text or if it needs correction. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a recorded API response that's being used as a test fixture. The ''s vs it's difference is part of the actual response from the API. Changing it would make the test fixture inaccurate to what the API actually returned. Test fixtures should preserve the exact data that was recorded, even if it contains what appears to be typos. Maybe the typo in the API response could cause issues for downstream consumers? Maybe we should document this quirk? No, the test fixture's job is to accurately record what the API returned. If there are issues with the API response format, that should be handled in the actual code, not by modifying test fixtures. Delete the comment. Test fixtures should preserve the exact recorded data, not modify it to fix perceived issues in the API response.
23. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_chat/test_ollama_streaming_chat_with_events_with_no_content.yaml:109
- Draft comment:
There's a typographical error in the assistant's streamed message. The chunk on line 109 shows "''s" which appears to be an incorrect representation of "it's". Please consider correcting this to maintain consistency in the joke's text. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a test cassette file that records actual API responses for testing purposes. The apparent "typo" is actually the real response from the API that we want to test against. Modifying this would defeat the purpose of having recorded test data. The file is meant to capture the actual API behavior, not idealized responses. Maybe the typo in the API response could cause issues in how the response is parsed or handled by the code? No - if there were parsing issues with this format, they should be handled in the actual code tests, not by modifying the recorded test data to be "cleaner" than reality. Delete this comment. We should not modify recorded API responses in test cassettes as they are meant to capture actual API behavior, not idealized responses.
24. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_generation/test_ollama_async_generation_legacy.yaml:26
- Draft comment:
Typographical error: The response message contains 'couldn''t' with two consecutive apostrophes. It should be 'couldn't' with a single apostrophe. - 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.
25. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_generation/test_ollama_async_generation_with_events_with_content.yaml:27
- Draft comment:
Typo: The word 'couldn''t' should be corrected to "couldn't" to maintain proper punctuation. - Reason this comment was not posted:
Comment looked like it was already resolved.
26. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_generation/test_ollama_async_generation_with_events_with_no_content.yaml:27
- Draft comment:
Typographical Error: In the JSON string on line 27, "couldn''t" should be "couldn't". Please fix the extra apostrophe. - Reason this comment was not posted:
Comment looked like it was already resolved.
27. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_generation/test_ollama_async_streaming_generation_legacy.yaml:58
- Draft comment:
Typographical Issue: On line 58, the response value "''s" looks suspicious and might be a typo (possibly intended as "Here's" or similar). Please verify the correct text. - 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.
28. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_generation/test_ollama_async_streaming_generation_with_events_with_content.yaml:57
- Draft comment:
It appears that the response token on lines 56-58 includes "''s". This looks like a typographical error and might be intended to form "Here's". Please verify and correct the quote usage. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a recorded API response from Ollama, not hand-written code. The double quotes are likely what the API actually returned. Modifying the response would make the test cassette inaccurate to what actually happened in the real API call. Test cassettes should reflect reality, not what we think the API should have returned. Maybe the API response is actually buggy and we should report this upstream to Ollama? Maybe this indicates a real issue that should be fixed? Even if there is an issue in Ollama's response formatting, the test cassette's job is to accurately record what happened, not to fix the upstream API's behavior. The test would become inaccurate if we modified the response. Delete the comment. Test cassettes should record actual API responses exactly as they occurred, not modify them to what we think they should be.
29. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_generation/test_ollama_async_streaming_generation_with_events_with_no_content.yaml:57
- Draft comment:
The response string appears to split the word "Here's" into 'Here' (line 56) and "''s" (line 57). Consider merging them into "Here's" to maintain the correct spelling and readability. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a test fixture file that captures actual API responses. The splitting of "Here's" into separate chunks is not a bug or issue - it's just how the LLM tokenized and streamed the response. Changing this would mean artificially modifying the real API response, which would defeat the purpose of having accurate test fixtures. The splitting doesn't affect functionality. Maybe having inconsistent token splits could make the tests less deterministic or harder to maintain over time? No, the token splits are determined by the LLM's tokenizer and are part of the real behavior we want to test. Artificially combining them would make the test less realistic. Delete this comment. The token splitting is expected behavior in streaming responses and doesn't need to be "fixed" in a test fixture.
30. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_generation/test_ollama_generation_with_events_with_content.yaml:26
- Draft comment:
Typographical error detected in the JSON response string: "Here''s one:" on line 26. It appears to use two apostrophes instead of one. Consider changing it to "Here's one:". - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The double apostrophe is actually correct YAML syntax for escaping single quotes within a single-quoted string. This is a test fixture file, and the response is properly encoded. The comment is suggesting a change that would actually break the YAML syntax. Additionally, this is a recorded API response - we shouldn't modify recorded test data. Could there be a case where single quotes would work better here? Maybe the test would be more readable with single quotes? No - this is a recorded API response in a test cassette. We should preserve the exact format of the recorded data, and the double apostrophe escaping is correct YAML syntax. Delete this comment as it suggests an incorrect change that would break valid YAML syntax in a test fixture file.
31. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_generation/test_ollama_generation_with_events_with_no_content.yaml:26
- Draft comment:
Typo in the response JSON string: It appears that in the text "Here''s one:", there is an extra single quote. Consider changing it to "Here's one:" for clarity. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a test fixture file that records actual API responses. The double single quotes are likely intentional for proper JSON string escaping. Since this is recorded data from an actual API response, we shouldn't modify it even if it looks unusual. Changing it could make the test invalid or cause it to not match real responses. Maybe the double quotes are actually a bug in the API response that should be fixed? Maybe this affects how the response is parsed? No, the response is clearly working as intended since it's a recorded successful API interaction. The test is passing with this data, so the escaping must be correct. The comment should be deleted because it suggests modifying recorded test data that accurately represents a real API response.
32. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_generation/test_ollama_streaming_generation_legacy.yaml:44
- Draft comment:
Typo detected in the response at line 44: the text "''s" looks like it might be an unintended double apostrophe. Consider checking if it should be "it's" or another form. - 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.
33. packages/opentelemetry-instrumentation-ollama/tests/cassettes/test_generation/test_ollama_streaming_generation_with_events_with_no_content.yaml:44
- Draft comment:
Typo: The response text on line 44 has "''s" which appears to have an extra apostrophe. If the intended contraction is "it's", consider correcting it to avoid confusion. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% This is a test cassette file that records actual API responses. The double apostrophe is part of the real response from the LLM. Since this is recorded data, we shouldn't modify it even if it contains typos - that would make the test data inaccurate. The comment is suggesting we change actual response data, which would be wrong. Maybe the double apostrophe indicates a bug in the LLM or the way responses are being recorded? Could this reveal an actual issue we should investigate? Even if there is an issue with the LLM's output or recording, the solution isn't to modify the test cassette - that would hide the real behavior. Any fixes would need to happen in the LLM or recording code itself. Delete this comment. We should not modify recorded test data to fix typos, as that would make the tests inaccurate. The double apostrophe is part of the actual API response that needs to be preserved.
Workflow ID: wflow_v43Y0XxQQyDHGehP
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
packages/opentelemetry-instrumentation-ollama/opentelemetry/instrumentation/ollama/__init__.py
Outdated
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
✅ PR Requirements
📌 Issue Requirements
Important
Adds event emission to Ollama instrumentation with a config option to toggle between legacy attributes and event-based logging, including tests for both modes.
__init__.py
.use_legacy_attributes
config inconfig.py
to toggle between legacy attributes and event-based logging._emit_message_events()
and_emit_choice_events()
in__init__.py
for event emission.use_legacy_attributes
andevent_logger
toConfig
inconfig.py
.MessageEvent
andChoiceEvent
inevent_handler.py
.emit_event()
,_emit_message_event()
, and_emit_choice_event()
inevent_handler.py
.is_content_enabled()
andshould_emit_events()
inutils.py
.test_chat.py
,test_embeddings.py
, andtest_generation.py
.conftest.py
for different logging modes.This description was created by
for 8de90b2. You can customize this summary. It will automatically update as commits are pushed.