Add HTTPWithContextFnOpts so requests can have eval-time context

[chrisberkhout]

Proposed commit message

Add HTTPWithContextFnOpts so requests can have eval-time context (#)

Allows propagation of eval-time context to the HTTP client by having
`httpLib` include a function that can provide the current context at
request time, rather than a fixed context.

Support in `cel-go` for context propagation[1][2] would make this
pattern unnecessary.

This is for the upcoming OTel tracing in the CEL input[3], which needs
HTTP request tracing to find parent span information in the context.

[1]: https://github.com/google/cel-go/issues/557
[2]: https://github.com/google/cel-go/pull/925
[3]: https://github.com/elastic/beats/pull/48440

[efd6]

Can you show me how this intended to be used? I've looked at elastic/beats#48440, but I don't see that doing anything that could not be done with the API as it stands since the context that is used via the closure is not altered by the closure, meaning that any operation on the context would propagate to all other instances of the value. Is there more do be done in that change that would help me understand why this is necessary?

[chrisberkhout]

@efd6

In the existing code, when HTTP requests from CEL are performed, it's like this:

func (l httpLib) get(url types.String) (*http.Response, error) {
	req, err := http.NewRequestWithContext(l.ctx, http.MethodGet, string(url), nil)
	...

with context put into httpLib by HTTPWithContextOpts, which the CEL Input uses in newProgram.

So requests always execute with the context from CEL program creation time. Multiple evaluations of the program will use the same old context for HTTP requests, and the OTel tracing span for the HTTP request won't be able to identify a particular evaluation as its parent.

Ideally, cel-go's ContextEval would have a way to make the context it's given available to functions, but currently that's not the case. It's discussed in google/cel-go#925 and related PRs.

So my workaround is to pass in a function at program creation time, and make sure it will return the right context during each eval.

In elastic/beats#48440 I pass in the function here:

ctxForEval := ctx
prg, ast, cov, err := newProgram(func() context.Context { return ctxForEval }, cfg.Program, root, getEnv(cfg.AllowedEnvironment), client, limiter, httpOptions, patterns, cfg.XSDs, log, trace, wantDump, doCov)

and make sure it'll return the right context for each eval here:

ctxForEval = execCtx
state, err = evalWith(execCtx, prg, ast, state, start, wantDump, budget-1)

When the HTTP request is performed, it'll call the function to get the current context, here:

func (l httpLib) get(url types.String) (*http.Response, error) {
	req, err := http.NewRequestWithContext(l.ctxFn(), http.MethodGet, string(url), nil)
	...

[efd6]

That's what I thought, but I'm not convinced it does what you are wanting.

In elastic/beats#48440 I pass in the function here:

ctxForEval := ctx
prg, ast, cov, err := newProgram(func() context.Context { return ctxForEval }, cfg.Program, root, getEnv(cfg.AllowedEnvironment), client, limiter, httpOptions, patterns, cfg.XSDs, log, trace, wantDump, doCov)

This results in a shared context. The context.Context documentation specifies that "Context's methods may be called by multiple goroutines simultaneously." This has two implications when considered with the stated intent for the type: 1. the methods must operate on references; and 2. the methods must be synchronised across concurrent access. The second part is not relevant here, but the first is very much so.

AFAICS for what you want to do, the context that is returned by the closure must be distinct in some way from the parent (exactly how is the thing that I'd like to understand). When you say, "return the right context during each eval", what is "the right context"?

As far as the implementation goes, I'd like not to expand the API like this. Given that context.Context is an interface (and also, that it implements the Value(key any) any method) there are two options that mean that we do not need to change the API surface like this. We can either:

1. Rather than adding an explicit signature for this, we can add the following code

   type RequestContexter inferface {
   	RequestContext() context.Context
   }
   
   func getTracer(ctx context.Context) context.Context {
   	rCtx, ok := ctx.(RequestContexter)
   	if ok {
   		return rCtx.RequestContext()
   	}
   	return ctx
   }
   

   and document that if the context.Context that is handed to HTTPWithContext satisfies RequestContexter, then the context.Context used in HTTP requests will be obtained by calling the RequestContext method. How this wrapping is done in the client code needs to consider the issue that I raise above.

2. Alternatively the closure can be stored as a context value.

I think the first of these is simpler and clearer.

[chrisberkhout]

The right context

AFAICS for what you want to do, the context that is returned by the closure must be distinct in some way from the parent (exactly how is the thing that I'd like to understand). When you say, "return the right context during each eval", what is "the right context"?

We start with:

ctx := ctxtool.FromCanceller(env.Cancelation)

Then inside the periodic run we do this (OTel puts trace/span information in):

runCtx, runSpan := otelTracer.Start(ctx, "cel.periodic.run")

Then before the individual evaluation we do:

execCtx, execSpan := otelTracer.Start(runCtx, "cel.program.execution")

And execCtx is the one I want to be used in CEL's HTTP requests - the same as what we're giving to cel-go's ContextEval function. When you say "parent", I assume you're referring to runCtx, which has tracing information for the parent span.

Using this branch the trace looks like this:

If naively using unmodified Mito those HTTP GET spans are created with the original ctx value that has no tracing information so they are root spans.

The issue

Is the issue you're raising that the interface type is copied when it is returned from the closure, so reads work but cancellation won't modify the copy? I think in theory a context.Context implementation could have that problem but in practice we're using implementations that have reference types for their underlying data. Also it would be a problem any time we pass/return context.Context rather than *context.Context, wouldn't it?

Still not sure if I've understood the issue. I want to get that clear before deciding how to proceed.

[efd6]

Thanks. That explains it, and I now see what you are doing. This complexity comes from the issues that were discussed here.

I would like not to do it with the closure and instead use the optional interface approach. Apart from not expanding the API surface and with careful construction, the data flow that I missed in the closure behaviour can be made explicit.

[chrisberkhout]

Okay, I'm trying to think through the options.

I see that adding HTTPWithContextFnOpts expands Mito's API, at least if the original HTTPWithContextOpts is retained. It doesn't need to be retained, but it's easy and makes it a non-breaking change. The API expansion adds functionality, and does it explicitly, by taking as an argument a function that returns the current context.

Your earlier suggestion would have that new function be provided by a new interface that is optionally implemented by the existing argument. Mito still gets expanded to handle both cases, but that's not explicit in its API. We couldn't mark the old way as deprecated because the new way is embedded in the old way.

We'd have the context provider we need wrapped in a different context that we don't need.

The optional interface would make more sense to me if it was extracting something from the type implementing it, and if it wasn't returning a different, unconnected instance of the same type.

Keeping the existing function signature doesn't seem to be better overall.

I was thinking, if cel.Program.ContextEval() was wrapped by Mito, then we could use the context given there, and how it's provided to the HTTP functions could be an internal implementation detail of Mito. But currently, Mito doesn't wrap that and the CEL Input uses cel-go directly to initiate the evaluation.

[efd6]

It doesn't need to be retained, but it's easy and makes it a non-breaking change.

This is important. We would need to make a v2 if it is removed.

We'd have the context provider we need wrapped in a different context that we don't need.

I'm not sure what you mean by this.

The optional interface would make more sense to me if it was extracting something from the type implementing it, and if it wasn't returning a different, unconnected instance of the same type.

I don't see this as a problem. If the interface were type ContextConditioner interface { ConditionContext() context.Context } then the name says what we are doing; providing a conditioned context to use.

I'm not sure what you mean in the last paragraph. By "Mito", do you mean the mito executable? If that it is important here to pass in a context to the eval, we can change that.

[chrisberkhout]

It doesn't need to be retained, but it's easy and makes it a non-breaking change.

This is important. We would need to make a v2 if it is removed.

I don't mind keeping it backwards compatible, but I think a new version would also be fine.
Beats could start using the new one. I'm not aware of any other uses. On pkg.go.dev it says "Imported by: 0". Github only finds Beats and Mito forks. If there are others they could stay on the old version or update with a small change.

We'd have the context provider we need wrapped in a different context that we don't need.

I'm not sure what you mean by this.

On the CEL Input side I have these different context values:

ctx := ctxtool.FromCanceller(env.Cancelation)
runCtx, runSpan := otelTracer.Start(ctx, "cel.periodic.run")
execCtx, execSpan := otelTracer.Start(runCtx, "cel.program.execution")

The execCtx value is already passed to cel-go's cel.Program.ContextEval.
We want httpLib functions to also use the execCtx value.
Why give it the older ctx value that we don't need with an optional interface to access the current execCtx value that it will actually use?
We can just give it the a function that returns the one value it should use.

The optional interface would make more sense to me if it was extracting something from the type implementing it, and if it wasn't returning a different, unconnected instance of the same type.

I don't see this as a problem. If the interface were type ContextConditioner interface { ConditionContext() context.Context } then the name says what we are doing; providing a conditioned context to use.

I don't think it's strictly wrong, just counter intuitive. The interface can explain itself but what it's doing still seems odd to me. Usually you pass forward a context and any changes are propagated forward and you always use the current one. It seems odd to have an older context value and call a method on that to get the current one.

I'm not sure what you mean in the last paragraph. By "Mito", do you mean the mito executable? If that it is important here to pass in a context to the eval, we can change that.

No, not that.
I don't think it's a way forward here, but I was just imagining that if the Mito library was designed as a wrapper around cel-go rather than a toolkit for working with it, then the CEL Input would initiate the evaluation through Mito and Mito would be able to do whatever it wants to get the provided context through to the HTTP functions, without it affecting its external interface.

[efd6]

I don't mind keeping it backwards compatible, but I think a new version would also be fine.

v2s are a pain. I'd really rather avoid it unless it's entirely necessary, which I do not believe is the case here.

Why give it the older ctx value that we don't need with an optional interface to access the current execCtx value that it will actually use?
We can just give it the a function that returns the one value it should use.

Except this is not the only thing we are doing. The code does that but the data flow is not immediately obvious. At least, it wasn't immediately obvious to me the first time I read the code. We are handing a context that we hold a reference to by virtue of a closure. This makes the data flow relatively opaque.

I don't think it's strictly wrong, just counter intuitive. The interface can explain itself but what it's doing still seems odd to me. Usually you pass forward a context and any changes are propagated forward and you always use the current one. It seems odd to have an older context value and call a method on that to get the current one.

Let's accept that that it's true that it's counter intuitive for the sake of argument (I'm unconvinced). In that case, the context.Context.Value can be used to get the trace context. With appropriate helpers this can get the same kind of clarity of data flow that I'm looking for and still not expand the set of functions that we need to be able to implement it.

I don't think it's a way forward here, but I was just imagining that if the Mito library was designed as a wrapper around cel-go rather than a toolkit for working with it, then the CEL Input would initiate the evaluation through Mito and Mito would be able to do whatever it wants to get the provided context through to the HTTP functions, without it affecting its external interface.

I still think I'm not understanding exactly what you are saying. I'm still not clear what "Mito" is; the relevant entities that exist are github.com/elastic/mito/lib, mito (the command in github.com/elastic/mito/cmd/mito) and github.com/elastic/beats/…/input/cel. The lib package is indeed a library and from a design perspective the only wrappers around cel-go are mito (the command) and the CEL input in Filebeat.

[chrisberkhout]

I don't think it's a way forward here, but I was just imagining that if the Mito library was designed as a wrapper around cel-go rather than a toolkit for working with it, then the CEL Input would initiate the evaluation through Mito and Mito would be able to do whatever it wants to get the provided context through to the HTTP functions, without it affecting its external interface.

I still think I'm not understanding exactly what you are saying. I'm still not clear what "Mito" is; the relevant entities that exist are github.com/elastic/mito/lib, mito (the command in github.com/elastic/mito/cmd/mito) and github.com/elastic/beats/…/input/cel. The lib package is indeed a library and from a design perspective the only wrappers around cel-go are mito (the command) and the CEL input in Filebeat.

This part isn't important, but for the sake of interest, I was just thinking that although the mito library fits in here:

if instead it was like this:

then it it could take that context provided for evaluation and make sure it gets to where it's needed in the HTTP functions, and that logic would be entirely internal to mito/lib, with no change for input/cel (or cel-go).

Let's accept that that it's true that it's counter intuitive for the sake of argument (I'm unconvinced). In that case, the context.Context.Value can be used to get the trace context. With appropriate helpers this can get the same kind of clarity of data flow that I'm looking for and still not expand the set of functions that we need to be able to implement it.

I wonder if there's still a disagreement about facts here that's leading us to different conclusions.
When you say "the context.Context.Value can be used to get the trace context" there are two things that come to mind:

1. The existing context.Context.Value doesn't contain anything about the trace. That's in a derived context created later.
2. "trace context" probably isn't the right term. Ultimately we want the calls to http.NewRequestWithContext to use a context that contains the current trace.Span where the OTel SDK code expects it. The HTTP spans are created automatically by OTel SDK code so new helpers to access trace information differently wouldn't help.

Why give it the older ctx value that we don't need with an optional interface to access the current execCtx value that it will actually use?
We can just give it the a function that returns the one value it should use.

Except this is not the only thing we are doing. The code does that but the data flow is not immediately obvious. At least, it wasn't immediately obvious to me the first time I read the code. We are handing a context that we hold a reference to by virtue of a closure. This makes the data flow relatively opaque.

I agree it's still not great, because without changes to cel-go we can't get a value directly from the initiation of the eval to the http functions. We have to go around.

"We are handing a context that we hold a reference to by virtue of a closure." - I think we're talking about either doing this directly as a different argument, or doing it as a method in an interface optionally implemented by the exiting argument. And the existing argument's own value will be ignored if it has the interface to get the alternate value.

Maybe there are other ways to make it more obvious, like using an interface or a struct or something to pass this reference, or a function passed in the other direction ("update context for httpLib" rather than "provide context for httpLib").

I'll come up with some minimal code that reproduces the scenario and then we'll definitely be on the same page and we can compare alternative solutions more easily.

[chrisberkhout]

Here's a simplified version of what's done in CEL Input (in the draft PR) and in mito/lib (existing code).
We already have the dependencies, so you can put it in the beats root dir and run it there.

There's a TODO comment showing what I'm aiming for: making the context for HTTP requests be the same as the one given to ContextEval.

I see the issue here as httpLib containing and using a context from compile time when it should use a context from eval or request time. Since cel-go doesn't let us pass the eval context through to the functions, we need a way to go around, which is not nice data flow, but hopefully can be made clear.

Does this help?

package main

import (
	"context"
	"fmt"
	"net/http"
	"os"

	"github.com/google/cel-go/cel"
	"github.com/google/cel-go/common/types"
	"github.com/google/cel-go/common/types/ref"
	sdktrace "go.opentelemetry.io/otel/sdk/trace"
	"go.opentelemetry.io/otel/trace"
)

////////////////////////////////////////////////////////////////////////////////
// Mito - simplified version of the code in mito/lib
////////////////////////////////////////////////////////////////////////////////

func HTTPWithContextOpts(ctx context.Context, client *http.Client, options HTTPOptions) cel.EnvOption {
	// simplification: remove initialization of client and limiter
	return cel.Lib(httpLib{
		client:  client,
		options: options,
		ctx:     ctx,
	})
}

type HTTPOptions any // simplification

type httpLib struct {
	client  *http.Client
	ctx     context.Context
	options HTTPOptions
}

func (l httpLib) CompileOptions() []cel.EnvOption { // simplifcation: only the get function
	return []cel.EnvOption{
		cel.Function("get",
			cel.Overload(
				"get_string",
				[]*cel.Type{cel.StringType},
				cel.MapType(cel.StringType, cel.DynType),
				cel.UnaryBinding(l.doGet), // simplification: removed catch() wrapper
			),
		),
	}
}
func (httpLib) ProgramOptions() []cel.ProgramOption { return nil } // simplification: no options

func (l httpLib) get(url types.String) (*http.Response, error) {
	// simplification: no error handling, auth or headers

	// TODO: make the context used for this request match the one provided to ContextEval
	req, _ := http.NewRequestWithContext(l.ctx, http.MethodGet, string(url), nil)

	// addition: assert that there's a valid span in the context to use as the parent
	if sc := trace.SpanFromContext(req.Context()).SpanContext(); sc.IsValid() {
		fmt.Println("SpanID in request context:", sc.SpanID())
	} else {
		fmt.Println("ERROR: No valid span in request context!")
		os.Exit(1)
	}

	return l.client.Do(req)
}
func (l httpLib) doGet(arg ref.Val) ref.Val {
	// simplification: do the get, but don't handle errors or the actual response
	l.get(arg.(types.String))
	return types.DefaultTypeAdapter.NativeToValue(make(map[string]interface{}))
}

////////////////////////////////////////////////////////////////////////////////
// CEL Input
////////////////////////////////////////////////////////////////////////////////

func main() {
	// setup
	otelTracerProvider := sdktrace.NewTracerProvider()
	otelTracer := otelTracerProvider.Tracer("github.com/elastic/beats/v7/x-pack/filebeat/input/cel")
	ctx := context.Background() // simplification

	// build the CEL program once

	opts := []cel.EnvOption{HTTPWithContextOpts(ctx, http.DefaultClient, new(HTTPOptions))}
	env, _ := cel.NewEnv(opts...)
	ast, _ := env.Compile("get('http://www.example.com/')")
	prg, _ := env.Program(ast)

	// execute CEL program - this will be done repeatedly

	runCtx, _ := otelTracer.Start(ctx, "cel.periodic.run")
	execCtx, execSpan := otelTracer.Start(runCtx, "cel.program.execution")
	fmt.Println("SpanID in ContextEval context:", execSpan.SpanContext().SpanID())
	prg.ContextEval(execCtx, map[string]any{})
}

My fix would give httpLib a way to get the context at request time. That way could be put into the existing compile-time context, not I'm not convinced that's better.

--- example.go.orig	2026-01-23 17:27:46.358648890 +0100
+++ example.go	2026-01-23 17:32:23.289202073 +0100
@@ -20,19 +20,27 @@
 func HTTPWithContextOpts(ctx context.Context, client *http.Client, options HTTPOptions) cel.EnvOption {
 	// simplification: remove initialization of client and limiter
 	return cel.Lib(httpLib{
 		client:  client,
 		options: options,
-		ctx:     ctx,
+		ctxFn:   func() context.Context { return ctx },
+	})
+}
+
+func HTTPWithContextFnOpts(ctxFn func() context.Context, client *http.Client, options HTTPOptions) cel.EnvOption {
+	return cel.Lib(httpLib{
+		client:  client,
+		options: options,
+		ctxFn:   ctxFn,
 	})
 }
 
 type HTTPOptions any // simplification
 
 type httpLib struct {
 	client  *http.Client
-	ctx     context.Context
+	ctxFn   func() context.Context
 	options HTTPOptions
 }
 
 func (l httpLib) CompileOptions() []cel.EnvOption { // simplifcation: only the get function
 	return []cel.EnvOption{
@@ -49,12 +57,11 @@
 func (httpLib) ProgramOptions() []cel.ProgramOption { return nil } // simplification: no options
 
 func (l httpLib) get(url types.String) (*http.Response, error) {
 	// simplification: no error handling, auth or headers
 
-	// TODO: make the context used for this request match the one provided to ContextEval
-	req, _ := http.NewRequestWithContext(l.ctx, http.MethodGet, string(url), nil)
+	req, _ := http.NewRequestWithContext(l.ctxFn(), http.MethodGet, string(url), nil)
 
 	// addition: assert that there's a valid span in the context to use as the parent
 	if sc := trace.SpanFromContext(req.Context()).SpanContext(); sc.IsValid() {
 		fmt.Println("SpanID in request context:", sc.SpanID())
 	} else {
@@ -80,17 +87,19 @@
 	otelTracer := otelTracerProvider.Tracer("github.com/elastic/beats/v7/x-pack/filebeat/input/cel")
 	ctx := context.Background() // simplification
 
 	// build the CEL program once
 
-	opts := []cel.EnvOption{HTTPWithContextOpts(ctx, http.DefaultClient, new(HTTPOptions))}
+	ctxForEval := ctx
+	opts := []cel.EnvOption{HTTPWithContextFnOpts(func() context.Context { return ctxForEval }, http.DefaultClient, new(HTTPOptions))}
 	env, _ := cel.NewEnv(opts...)
 	ast, _ := env.Compile("get('http://www.example.com/')")
 	prg, _ := env.Program(ast)
 
 	// execute CEL program - this will be done repeatedly
 
 	runCtx, _ := otelTracer.Start(ctx, "cel.periodic.run")
 	execCtx, execSpan := otelTracer.Start(runCtx, "cel.program.execution")
 	fmt.Println("SpanID in ContextEval context:", execSpan.SpanContext().SpanID())
-	prg.ContextEval(execCtx, map[string]any{})
+	ctxForEval = execCtx
+	prg.ContextEval(ctxForEval, map[string]any{})
 }

[efd6]

This part isn't important, but for the sake of interest, I was just thinking that although the mito library fits in here:

if instead it was like this:

then it it could take that context provided for evaluation and make sure it gets to where it's needed in the HTTP functions, and that logic would be entirely internal to mito/lib, with no change for input/cel (or cel-go).

This would require that mito/... know about Filebeat inputs taking the design to be more akin to a framework than a library. This is something that was intentionally avoided in the design that exists.

I wonder if there's still a disagreement about facts here that's leading us to different conclusions.
When you say "the context.Context.Value can be used to get the trace context" there are two things that come to mind:

1. The existing context.Context.Value doesn't contain anything about the trace. That's in a derived context created later.

Yes, that's understood. The value that is handed to the context in the model I'm thinking of would behave almost exactly the same way that the closed-over value is handled now, just via the context.Context.Value call, and with helper functions to make the data flow more obvious.

2. "trace context" probably isn't the right term. Ultimately we want the calls to http.NewRequestWithContext to use a context that contains the current trace.Span where the OTel SDK code expects it. The HTTP spans are created automatically by OTel SDK code so new helpers to access trace information differently wouldn't help.

My point is that it's a context that is used to handle tracing. I think this is captured by the name I used.

I'll take a look at the alternative code you have when I'm back, but I see that it still uses the closure. I'll put together a version that shows how this can be avoided.

[chrisberkhout]

This would require that mito/... know about Filebeat inputs taking the design to be more akin to a framework than a library. This is something that was intentionally avoided in the design that exists.

👍

My point is that it's a context that is used to handle tracing. I think this is captured by the name I used.

Okay. It's also the context used to handle cancellation of the evaluation.

I'll take a look at the alternative code you have when I'm back, but I see that it still uses the closure. I'll put together a version that shows how this can be avoided.

Thanks. Yeah, I think it's good to compare alternate implementations now. That code and diff in my last comment was just reproducing what's in the original PR, but showing both sides in a shorter example.

I think these are the options we're looking at:

0. Existing code: store the compile time context, use it in requests.
1. Current PR: store a function that produces the eval time context, use that in requests.
2. Suggestion: continue to store the compile time context, using an interface optionally implemented on the compile time context retrieve the eval time context, use that in requests.

I see that option 2 preserves the function signature of HTTPWithContextOpts. If building from scratch I don't think it would make sense for the compile time context to be stored at all. Option 1 is bad because context is propagated out-of-band rather than through the call stack as usual, but that's necessary because of cel-go. Option 2 is bad in the same way, plus it buries the eval time context inside in a compile time context that has no other use.

[chrisberkhout]

I looked at the diffs you sent @efd6, thanks.

They showed what I expected the optional interface approach to be (option 2 from my last comment).

Can you say why that is better?

[efd6]

Can you say why that is better?

The reason that I prefer it is that it doesn't proliferate function signatures.

[chrisberkhout]

Ok. I prefer the additional function signature because:

• It says what it does - there's no need for a comment to explain that the context in the ctx argument will not be used if it implements an interface to provide a different value.
• It provides the current context value without attaching it to a redundant, unrelated context value.
• It doesn't store a redundant context value in httpLib.

I have two ideas:

• We can get someone else's opinion and do that.
• We can leave mito as is an do the context swapping in an early round tripper added to the client.

[efd6]

If we can do it without altering the lib, I would prefer that.

[chrisberkhout]

Closing in favor of an approach that can be done with changes the CEL input only.