Instrumentation genai langchain #1
Conversation
…entelemetry-python-contrib into instrumentation-genai-langchain
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| __version__ = "0.0.1" No newline at end of file |
There was a problem hiding this comment.
Files should have a newline at the end
| span.set_attribute( | ||
| ErrorAttributes.ERROR_TYPE, type(error).__qualname__ | ||
| ) | ||
| self._end_span(run_id) No newline at end of file |
There was a problem hiding this comment.
Files should have a newline at the end
| TraceContextTextMapPropagator().inject(extra_headers, context=ctx) | ||
| kwargs["extra_headers"] = extra_headers | ||
|
|
||
| return wrapped(*args, **kwargs) No newline at end of file |
There was a problem hiding this comment.
Files should have a newline at the end
|
|
||
| python-dotenv[cli] | ||
|
|
||
| # For local development: `pip install -e /path/to/opentelemetry-instrumentation-langchain` No newline at end of file |
There was a problem hiding this comment.
Files should have a newline at the end
| LangChainInstrumentor().uninstrument() | ||
|
|
||
| if __name__ == "__main__": | ||
| main() No newline at end of file |
There was a problem hiding this comment.
Files should have a newline at the end
| # Change to 'false' to hide prompt and completion content | ||
| OTEL_INSTRUMENTATION_LANGCHAIN_CAPTURE_MESSAGE_CONTENT=true | ||
|
|
||
| OTEL_SERVICE_NAME=opentelemetry-python-langchain No newline at end of file |
There was a problem hiding this comment.
Files should have a newline at the end
| # Change to 'false' to hide prompt and completion content | ||
| OTEL_INSTRUMENTATION_LANGCHAIN_CAPTURE_MESSAGE_CONTENT=true | ||
|
|
||
| OTEL_SERVICE_NAME=opentelemetry-python-langchain No newline at end of file |
There was a problem hiding this comment.
Files should have a newline at the end
| from langchain_core.messages import HumanMessage, SystemMessage | ||
| from langchain_openai import ChatOpenAI | ||
|
|
||
| import pytest |
There was a problem hiding this comment.
Imports should be core first, then packages, then local
|
Hi Ridhima, thank you for taking this on. Some overall thoughts: This PR is large at 24 files and 1700 lines, which is great because it's got lots of functionality needed by customers, but larger PRs are harder to review. Should we try to make it smaller? What are some strategies to do so? We could use this diff as a reference and build a from-scratch implementation in small increments. However, because this PR is open against a fork of the upstream Contrib repo, if we go with an incremental approach, after we fine-tune, approve, and merge our small PRs against this fork, we're going to be almost back to square one, because our final implementation will need to get offered upstream as a PR to the original Contrib repo. And now we have to either attempt to get a large PR reviewed and merged in a timely manner, or upstream Contrib will ask us to break our diff into multiple small PRs again. What do you think? |
|
|
||
| return span | ||
|
|
||
| def _end_span(self, run_id: UUID): |
There was a problem hiding this comment.
When I run this instrumentor locally, I don't see telemetry. I am seeing it start spans, but it doesn't end them -- this method never gets called for me.
|
@pmcollins thanks for taking a look and providing your suggestions. Also what is recommended, PRs against fork or against actual repo? If former, can we merge one PR at a time all the way to upstream ? |
|
Either one is fine with me -- as a reviewer I prefer a larger number of smaller PRs, so I would lean towards the first approach 😉. We can play it by ear. We can review and merge code here against your fork to experiment and discuss ideas, but eventually we're going to have to open PRs against upstream Contrib. As you suggest, we could start with a skeletal PR of just the minimal project files, and open it against upstream Contrib. (I've never created a new instrumentation so it will be a learning experience for me as well.) |
3 participants
Description
Instrument langchain invoking ChatOpenAI client. Produces spans, metrics and events.
Added tests for the same.
Note: some attributes string literals need addition to semcov.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.