feat(instrumentation-langchain): add initial package skeleton for instrumentation-langchain #3132
Conversation
yiyuan-he
force-pushed
the
skeleton-instrumentation-langchain
branch
from
952c0ef to
7a2c838
Compare
trentm
changed the title
|
|
||
| | Option | Type | Default | Description | | ||
| |--------|------|---------|-------------| | ||
| | `captureMessageContent` | `boolean` | `false` | Capture prompt and completion content. Can also be set via `OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT` environment variable. | |
There was a problem hiding this comment.
Is this env variable available in the specification? is it used in different places right now?
There was a problem hiding this comment.
This env var is used in instrumentation-openai. I'll remove it from this PR to keep the skeleton minimal and focused on structure. Will add the env var back along with supporting implementation (for message content capture) in a follow-up PR.
|
|
||
| ## Semantic Conventions | ||
|
|
||
| This package uses OpenTelemetry Semantic Conventions for GenAI operations. |
There was a problem hiding this comment.
It would be good to add specific semantic convention version planned for this, latest I assume?
https://github.com/open-telemetry/semantic-conventions/blob/v1.38.0/docs/README.md
There was a problem hiding this comment.
Added v1.38.0, thanks!
| }, | ||
| "devDependencies": { | ||
| "@opentelemetry/api": "^1.3.0", | ||
| "@opentelemetry/contrib-test-utils": "^0.52.2", |
There was a problem hiding this comment.
"@opentelemetry/contrib-test-utils": "^0.55.0",
"@opentelemetry/sdk-logs": "^0.208.0",
|
|
||
| export class LangChainInstrumentation extends InstrumentationBase<LangChainInstrumentationConfig> { | ||
| constructor(config: LangChainInstrumentationConfig = {}) { | ||
| super(PACKAGE_NAME, PACKAGE_VERSION, config); |
There was a problem hiding this comment.
Do you want to add code to handle env variable you mentioned in readme?
There was a problem hiding this comment.
Decided to remove the env var for now to facilitate a cleaner review. Related
| "packages/instrumentation-knex": {}, | ||
| "packages/instrumentation-koa": {}, | ||
| "packages/instrumentation-langchain": { | ||
| "skip-github-release": true |
There was a problem hiding this comment.
It would be good to ensure this would not be tried to be released as is currently a not fully functional instrumentation, I do not know if this will be sufficient
There was a problem hiding this comment.
I wasn't sure how to verify this since i'm not too familiar with this repo. However, i looked through examples/bunyan/package.json and found the "private": true configuration which should add another layer of safety by preventing npm publish. Let me know if you think this will not be sufficient though.
There was a problem hiding this comment.
private: true works for npm, that's how we usually do it.
This here makes sure we don't see it in the GitHub releases.
That's the two places where we would like to avoid publishing things for now, so I think this works 👍
|
It seems there is an error in the dependency tree. Error in log shows "npm error Could not resolve dependency: |
JacksonWeber
left a comment
JacksonWeber
left a comment
There was a problem hiding this comment.
Apart from Hector's comment regarding ensuring that this does not get tied to releases, LGTM
5 participants
Which problem is this PR solving?
This PR adds the initial project skeleton to kickstart a new
instrumentation-langchainpackage.Original PR raised by our intern: #3024
Short description of the changes
Mostly package metadata and licensing information. @mxiamxia and I (@yiyuan-he) also volunteer to be component owners for this new package.