Skip to content
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

Why tracer:http_headers_inject() instead of tracer:inject() passing type? #4

Open
james-callahan opened this issue Aug 9, 2018 · 8 comments

Comments

@james-callahan
Copy link
Collaborator

Other language seem to have a single inject method where you pass in the type. Why do we have different methods per type?

@rnburn
Copy link
Collaborator

rnburn commented Aug 9, 2018

There was a decision to get rid of the extra format parameter in the inject/extract method and bundle them into the carrier (see discussion on opentracing/opentracing-go#127).

The methods are similar to those in the C API, but I'm open to other designs.

@james-callahan
Copy link
Collaborator Author

So I sort of liked the more abstract inject API: lua has no 'standard' data type for http headers. Some frameworks use a simple table with key->value. Others add metamethods for case normalisation. Other environments use a richer object such as https://daurnimator.github.io/lua-http/0.2/#http.headers
This makes it unclear what should get provided to the http_headers_inject method.

@rnburn
Copy link
Collaborator

rnburn commented Aug 9, 2018

The carrier passed in just needs to store the key-values; it shouldn't do transformations. It's the responsibility of the tracer implementation to deal with the case sensitivity of header keys and ensure that they're valid.

@james-callahan
Copy link
Collaborator Author

The carrier passed in just needs to store the key-values;

That doesn't answer the issue though: e.g. do they get stored with carrier[key] = value or with carrier:add(key, value). And for an extractor: does it use e.g. carrier[key] or carrier:get(key)

@rnburn
Copy link
Collaborator

rnburn commented Aug 9, 2018

I would think carrier[key] = value and carrier[key] so that you can use a standard table for the carrier. You can always change the metamethods if you want to do something different.

@daurnimator
Copy link

daurnimator commented Aug 9, 2018

I would think carrier[key] = value and carrier[key] so that you can use a standard table for the carrier. You can always change the metamethods if you want to do something different.

It would be following an anti-pattern to have a dummy table that has __newindex and __index methods instead of passing in a function that is a getter/setter.

@rnburn
Copy link
Collaborator

rnburn commented Aug 9, 2018

The PHP and JavaScript APIs both use associative arrays for carriers.

A carrier interface more like what's in Go is also a possibility. But given that you can always use an intermediary table to propagate to a custom data structure, is there much of a use case for custom carrier implementations?

@james-callahan
Copy link
Collaborator Author

is there much of a use case for custom carrier implementations?

I think so: e.g. in LuaJIT, iterating over a table (with pairs/next) bails out of the JIT compiler: if the HTTP implementation wants to be fast, then they can't use a table for storing headers.

A different use-case is in WAFs: you want to be able to track order of headers, and take action on multiple occurrences of the same field (sometimes you want to allow it; other times it needs to be blocked). Using a plain dictionary for this structure doesn't work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants