-
Notifications
You must be signed in to change notification settings - Fork 15
feat: Support proxy for debugger endpoint v2 #918
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
base: main
Are you sure you want to change the base?
Conversation
bottlecap/src/traces/trace_agent.rs
Outdated
| .route(LLM_OBS_SPANS_ENDPOINT_PATH, post(Self::llm_obs_spans_proxy)) | ||
| .route(DEBUGGER_ENDPOINT_PATH, post(Self::debugger_logs_proxy)) | ||
| .route(V1_DEBUGGER_ENDPOINT_PATH, post(Self::debugger_logs_proxy)) | ||
| .route(V2_DEBUGGER_ENDPOINT_PATH, post(Self::debugger_logs_proxy)) |
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.
Is it correct to use Self::debugger_logs_proxy? Should any param here be different?
datadog-lambda-extension/bottlecap/src/traces/trace_agent.rs
Lines 388 to 398 in 0a5e11e
| async fn debugger_logs_proxy(State(state): State<ProxyState>, request: Request) -> Response { | |
| Self::handle_proxy( | |
| state.config, | |
| state.proxy_aggregator, | |
| request, | |
| "http-intake.logs", | |
| DEBUGGER_LOGS_INTAKE_PATH, | |
| "debugger_logs", | |
| ) | |
| .await | |
| } |
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.
Sorry, I was incorrect in Slack. After looking at the datadog-agent code, it looks like the params changed in v2. See the suggested code changes in the other comments
bottlecap/src/traces/trace_agent.rs
Outdated
| const LLM_OBS_EVAL_METRIC_INTAKE_PATH: &str = "/api/intake/llm-obs/v1/eval-metric"; | ||
| const LLM_OBS_EVAL_METRIC_INTAKE_PATH_V2: &str = "/api/intake/llm-obs/v2/eval-metric"; | ||
| const PROFILING_INTAKE_PATH: &str = "/api/v2/profile"; | ||
| const DEBUGGER_LOGS_INTAKE_PATH: &str = "/api/v2/logs"; |
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.
| const V1_DEBUGGER_LOGS_INTAKE_PATH: &str = "/api/v2/logs"; | |
| const V2_DEBUGGER_INTAKE_PATH: &str = "/api/v2/debugger"; |
nhulston
left a comment
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.
Looks good. Can we test in a real Lambda before merging?
|
Tested. Will merge after v89 is released. |
Now we support proxying requests from
/debugger/v1/input. This PR adds/debugger/v2/inputand/debugger/v1/diagnostics.#925
Test
Steps
DD_TRACE_DEBUGtotrueResult
Before:
debugger::unsupported_agentUnsupported Datadog agent detected. Snapshots from Dynamic Instrumentation/Exception Replay/Code Origin for Spans will not be uploaded. Please upgrade to version 7.49.0 or laterAfter:
Detected /debugger/v2/input endpointNotes
Thanks @nhulston @joeyzhao2018 @purple4reina for discussion and helping debug.