feat(bedrock): attach Dify app_id as Converse requestMetadata (opt-in)#3201
Draft
mas-sakai wants to merge 12 commits into
Draft
feat(bedrock): attach Dify app_id as Converse requestMetadata (opt-in)#3201mas-sakai wants to merge 12 commits into
mas-sakai wants to merge 12 commits into
Conversation
Add `_metadata.py` with `normalize_metadata_value` and `build_dify_request_metadata` helpers that produce Bedrock-compliant requestMetadata dicts. UUID app_ids pass through unchanged; other strings are restricted to `[a-zA-Z0-9\s:_@$#=/+,-.]` (per the Converse API constraint) and truncated to 256 characters. Mixed case is preserved — unlike Vertex AI labels, Bedrock requestMetadata accepts uppercase. Refs: langgenius/dify#35772, langgenius/dify-plugin-sdks#311 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In `_generate_with_converse`, when the `enable_request_metadata`
credential is set to `enabled`, read `app_id` from the current Dify
session (via `dify_plugin.get_current_session`) and attach it as
`{dify_app_id, dify_source}` on the Converse API `requestMetadata`
field. CloudWatch invocation logs then carry the marker, enabling
per-app filtering for cost and usage tracking.
Scoped to the Converse route only — the legacy InvokeModel path does
not support requestMetadata. `dify_plugin` is imported lazily and
wrapped in `try/except ImportError` so older SDK versions degrade
gracefully (no metadata attached). Tests cover the normalize helper
and `build_dify_request_metadata` short-circuits.
Refs: langgenius/dify#35772, langgenius/dify-plugin-sdks#311
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Surface `enable_request_metadata` as a provider-level select (default `disabled`) in `provider/bedrock.yaml`. When enabled, the Converse route attaches Dify metadata to Bedrock requestMetadata for CloudWatch log filtering. Description is on the `help` field so it renders below the input in the Dify console. Refs: langgenius/dify#35772, langgenius/dify-plugin-sdks#311 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Refs: langgenius/dify#35772, langgenius/dify-plugin-sdks#311 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Point `dify_plugin` at the `feat/pass-session-to-model-plugins` branch of `ryuta-kobayashi-ug/dify-plugin-sdks` (PR langgenius#313) so the Converse route can import `get_current_session()`. The branch SDK is version 0.9.0, which requires `tiktoken>=0.12.0` — bump bedrock's tiktoken constraint accordingly. Refresh `uv.lock`. This pin is temporary for the review period; revert to a release-version constraint once langgenius#313 is merged and a new SDK is published. Refs: langgenius/dify#35772, langgenius/dify-plugin-sdks#311 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds support for attaching Dify metadata (such as dify_app_id and dify_source) to Amazon Bedrock Converse API requests as requestMetadata for CloudWatch logging, controlled by a new enable_request_metadata credential setting. The review feedback suggests catching generic exceptions instead of just ImportError during session retrieval to prevent crashes, adding Chinese translations for the new credential configuration, and ensuring robust string conversion in the metadata normalization helper.
…view Broaden the exception catch around dify_plugin.get_current_session() to `Exception` so that contextvars LookupError, RuntimeError, or any other runtime issue in best-effort telemetry cannot break model generation (previously only ImportError was caught, which left the runtime path unprotected). In normalize_metadata_value, coerce non-string input via str() before the empty-check so numeric `0` becomes "0" instead of being dropped as falsy. Adds a regression test. Refs: langgenius/dify#35772, langgenius/dify-plugin-sdks#311 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Surface zh_Hans copy for the new credential's label, enabled/disabled option labels, and help text, and switch help to the `help.text.<lang>` form used by sibling entries (endpoint_url, proxy_url) so structure is consistent across the provider schema. Refs: langgenius/dify#35772, langgenius/dify-plugin-sdks#311 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolve conflicts in bedrock pyproject/uv.lock by taking upstream's boto3/botocore/tiktoken bumps while retaining the sdks#313 branch pin for dify_plugin (get_current_session is not in a released SDK yet).
16 tasks
…questMetadata per review - Widen `normalize_metadata_value` / `build_dify_request_metadata` type hints to `Any` to match the str() coercion the implementations already do. - Reject only `None` and `""` in `build_dify_request_metadata`; other falsy values (e.g. numeric 0) flow through to normalization rather than being silently dropped, matching the design intent for the helper. - Centralize the session lookup + metadata injection in a new `apply_dify_request_metadata_if_enabled` helper that merges into existing `parameters["requestMetadata"]` instead of overwriting it. Aligns with the OpenAI plugin's `apply_dify_metadata_if_enabled` structure. - Add tests covering the merge-with-existing path, the non-dict fallback, the disabled/missing credential no-ops, the session-lookup failure swallowing, and the falsy-but-non-empty input case. Refs: langgenius/dify#35772, langgenius/dify-plugin-sdks#311 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Marketplace rejects duplicate versions; main is at 0.0.67, so bump to 0.0.68. uv.lock already tracks the dify_plugin git+ fork; rerunning uv lock confirms it stays resolved against sdks langgenius#313. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applies the same ordering fix discussed on langgenius#3168 to the Bedrock helper: slice down to 256 chars before the regex sub() so pathological inputs do not incur unbounded regex work. The character set used by sub() replaces 1:1 by length, so the trailing truncation is no longer needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
11 tasks
…rmalize per review Per gemini-code-assist on langgenius#3233. Aligns 4 plugins on the same 'no side effects on existing args' principle. Refs: langgenius/dify#35772, langgenius/dify-plugin-sdks#311 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mas-sakai
had a problem deploying
to
models/bedrock
GitHub Actions
Failure
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Add opt-in support for forwarding the Dify
app_idto Amazon Bedrock's Converse APIrequestMetadatafield so CloudWatch invocation logs can be filtered per Dify app for cost and usage tracking.enable_request_metadata(select, defaultdisabled) exposed inprovider/bedrock.yaml._generate_with_conversereadsapp_idfrom the current Dify session viadify_plugin.get_current_session()and attaches{dify_app_id, dify_source}torequestMetadataon Converse / ConverseStream calls.models/llm/_metadata.pywithnormalize_metadata_value(sanitizes to the Bedrock-allowed character set[a-zA-Z0-9\s:_@$#=/+,-.], truncates to 256 chars, preserves mixed case) andbuild_dify_request_metadata(returnsNonefor emptyapp_idso the caller can skip attaching).InvokeModelpath does not supportrequestMetadataand is intentionally untouched.dify_pluginis imported lazily inside atry/except ImportError, so SDK versions withoutget_current_session(currently anything before sdks#313 ships) just skip metadata attachment without raising.0.0.66→0.0.67.Why opt-in
Default
disabledto preserve current behavior for tenants who don't want Dify identifiers landing in their CloudWatch logs. Operators opt in per-credential in the Dify console.Temporary SDK pin
pyproject.tomlpointsdify_pluginat thefeat/pass-session-to-model-pluginsbranch ofryuta-kobayashi-ug/dify-plugin-sdks(PR langgenius/dify-plugin-sdks#311 / commit reference #313) becauseget_current_session()is not yet available in a released SDK. The branch SDK is0.9.0and requirestiktoken>=0.12.0, so the bedrocktiktokenconstraint is bumped accordingly anduv.lockis refreshed.Action item before merge: once the SDK PR is merged and a new release is published, revert this branch pin back to a normal release-version constraint. Calling out explicitly so it isn't shipped as-is.
Files
models/bedrock/provider/bedrock.yaml— newenable_request_metadatacredential entry.models/bedrock/models/llm/_metadata.py— new helpers + tests.models/bedrock/models/llm/llm.py— gated metadata attachment in_generate_with_converse.models/bedrock/tests/test_metadata.py— unit tests for the normalize + build helpers.models/bedrock/manifest.yaml— version bump.models/bedrock/pyproject.toml/uv.lock— temporary SDK branch pin + tiktoken bump.Test plan
pytest models/bedrock/tests/test_metadata.py— UUID passthrough, invalid-char replacement, allowed-special-char preservation, mixed-case preservation, 256-char truncation, empty input, non-ASCII replacement,None/""short-circuit, source marker present.enable_request_metadata, invoke a Bedrock Converse model, and verify the CloudWatch model-invocation log entry carriesdify_app_idmatching the calling app anddify_source=dify.enable_request_metadata=disabled(default), confirm norequestMetadatafield is sent on the Converse request (regression check via packet capture or model invocation logs).dify_pluginSDK that lacksget_current_session, confirm the plugin still loads and Converse calls succeed without metadata (graceful degradation).Refs
🤖 Generated with Claude Code