You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@trafficserver.apache.org by Leif Hedstrom <zw...@apache.org> on 2014/03/01 00:21:14 UTC
[API Review] Expose HttpDebugNames:: to plugins
The internals of ATS has some static functions to convert from internal enum values to a descriptive string. This includes:
HttpDebugNames::get_server_state_name()
HttpDebugNames::get_event_name()
HttpDebugNames::get_action_name()
HttpDebugNames::get_cache_action_name()
HttpDebugNames::get_api_hook_name()
I would like to expose these (and any future such Debug functions) to plugin APIs. A new one that would come to mind is a “id to overridable configuration” function (but, that’s for later). I think there’s at least two ways to do this, the first would be e.g.
typedef enum
{
TS_HTTP_DEBUG_NAMES_NULL = -1,
TS_HTTP_DEBUG_NAMES_SERVER_STATE,
TS_HTTP_DEBUG_NAMES_EVENT_NAME,
TS_HTTP_DEBUG_NAMES_ACTION_NAME,
TS_HTTP_DEBUG_NAMES_CACHE_ACTION_NAME,
TS_HTTP_DEBUG_NAMES_API_HOOK_NAME,
TS_HTTP_DEBUG_NAMES_LAST_ENTRY
} TSHttpDebugNamesType;
tsapi const char* TSHttpDebugNameGet(TSHttpDebugNamesType name, int value);
This has the advantage that it’s really easy to extend with future additions, easy to use / remember, and trivial to implement. The downside is there’s no type safety on the “value” parameter other than the fact that the underlying methods (see above) deals with them.
The other alternative is to add one C function for each type, e.g.
tsapi const char* TSHttpServerStateNameGet(TSServerState value);
tsapi const char* TSHttpEventNameGet(TSEvent value);
tsapi const char* TSHttpActionNameGet(int value);
This has better type safety (in most cases), but makes it slightly harder to use and extend.
Please discuss which would be preferable. There’s currently no compatibility concerns regarding this addition, it’s a brand spanking new API. But we should design it for future compatibility.
Cheers,
— Leif
Re: [API Review] Expose HttpDebugNames:: to plugins
Posted by Leif Hedstrom <zw...@apache.org>.
From the feedback, I’d like to move along with the following two API additions:
tsapi const char* TSHttpServerStateNameLookup(TSServerState state);
tsapi const char* TSHttpHookNameLookup(TSHttpHookID hook);
tsapi const char* TSHttpEventNameLookup(TSEvent event);
The other 2 debug APIs in the core are not easily exposable, and probably provide little usable debugging feature (but we can revisit that later).
Let me know if there are any objections to this.
— Leif
Re: [API Review] Expose HttpDebugNames:: to plugins
Posted by Leif Hedstrom <zw...@apache.org>.
> On Mar 1, 2014, at 3:01 PM, James Peach <jp...@apache.org> wrote:
>
>> On Feb 28, 2014, at 3:21 PM, Leif Hedstrom <zw...@apache.org> wrote:
>>
>> The internals of ATS has some static functions to convert from internal enum values to a descriptive string. This includes:
>>
>> HttpDebugNames::get_server_state_name()
>> HttpDebugNames::get_event_name()
>> HttpDebugNames::get_action_name()
>> HttpDebugNames::get_cache_action_name()
>> HttpDebugNames::get_api_hook_name()
>>
>>
>> I would like to expose these (and any future such Debug functions) to plugin APIs. A new one that would come to mind is a “id to overridable configuration” function (but, that’s for later). I think there’s at least two ways to do this, the first would be e.g.
>>
>> typedef enum
>> {
>> TS_HTTP_DEBUG_NAMES_NULL = -1,
>> TS_HTTP_DEBUG_NAMES_SERVER_STATE,
>> TS_HTTP_DEBUG_NAMES_EVENT_NAME,
>> TS_HTTP_DEBUG_NAMES_ACTION_NAME,
>> TS_HTTP_DEBUG_NAMES_CACHE_ACTION_NAME,
>> TS_HTTP_DEBUG_NAMES_API_HOOK_NAME,
>> TS_HTTP_DEBUG_NAMES_LAST_ENTRY
>> } TSHttpDebugNamesType;
>>
>> tsapi const char* TSHttpDebugNameGet(TSHttpDebugNamesType name, int value);
>>
>>
>> This has the advantage that it’s really easy to extend with future additions, easy to use / remember, and trivial to implement. The downside is there’s no type safety on the “value” parameter other than the fact that the underlying methods (see above) deals with them.
>>
>>
>> The other alternative is to add one C function for each type, e.g.
>>
>> tsapi const char* TSHttpServerStateNameGet(TSServerState value);
>> tsapi const char* TSHttpEventNameGet(TSEvent value);
>> tsapi const char* TSHttpActionNameGet(int value);
>>
>>
>> This has better type safety (in most cases), but makes it slightly harder to use and extend.
>>
>>
>> Please discuss which would be preferable. There’s currently no compatibility concerns regarding this addition, it’s a brand spanking new API. But we should design it for future compatibility.
>
> I prefer the function-based API; it's simpler to describe and use. I don't mind the suggested naming convention, but consider TSHttpServerStateNameLookup(), which would match TSHttpHdrReasonLookup().
I like it, I'll make a new proposal as soon as I land.
>
> What values would TSHttpActionNameGet() accept?
>
Int :) it's a mess here, and we have a few bugs on this as well, from Nick I think. Can't look em up here though ...
I would probably punt on that portion of the APIs until the types for this is resolved. Extending later is easier than breaking compatibility IMO.
-- Leif
> J
Re: [API Review] Expose HttpDebugNames:: to plugins
Posted by Bryan Call <bc...@yahoo-inc.com>.
On 3/1/14, 9:01 PM, James Peach wrote:
> On Feb 28, 2014, at 3:21 PM, Leif Hedstrom <zw...@apache.org> wrote:
>
>> The internals of ATS has some static functions to convert from internal enum values to a descriptive string. This includes:
>>
>> HttpDebugNames::get_server_state_name()
>> HttpDebugNames::get_event_name()
>> HttpDebugNames::get_action_name()
>> HttpDebugNames::get_cache_action_name()
>> HttpDebugNames::get_api_hook_name()
>>
>>
>> I would like to expose these (and any future such Debug functions) to plugin APIs. A new one that would come to mind is a “id to overridable configuration” function (but, that’s for later). I think there’s at least two ways to do this, the first would be e.g.
>>
>> typedef enum
>> {
>> TS_HTTP_DEBUG_NAMES_NULL = -1,
>> TS_HTTP_DEBUG_NAMES_SERVER_STATE,
>> TS_HTTP_DEBUG_NAMES_EVENT_NAME,
>> TS_HTTP_DEBUG_NAMES_ACTION_NAME,
>> TS_HTTP_DEBUG_NAMES_CACHE_ACTION_NAME,
>> TS_HTTP_DEBUG_NAMES_API_HOOK_NAME,
>> TS_HTTP_DEBUG_NAMES_LAST_ENTRY
>> } TSHttpDebugNamesType;
>>
>> tsapi const char* TSHttpDebugNameGet(TSHttpDebugNamesType name, int value);
>>
>>
>> This has the advantage that it’s really easy to extend with future additions, easy to use / remember, and trivial to implement. The downside is there’s no type safety on the “value” parameter other than the fact that the underlying methods (see above) deals with them.
>>
>>
>> The other alternative is to add one C function for each type, e.g.
>>
>> tsapi const char* TSHttpServerStateNameGet(TSServerState value);
>> tsapi const char* TSHttpEventNameGet(TSEvent value);
>> tsapi const char* TSHttpActionNameGet(int value);
>>
>>
>> This has better type safety (in most cases), but makes it slightly harder to use and extend.
>>
>>
>> Please discuss which would be preferable. There’s currently no compatibility concerns regarding this addition, it’s a brand spanking new API. But we should design it for future compatibility.
> I prefer the function-based API; it's simpler to describe and use. I don't mind the suggested naming convention, but consider TSHttpServerStateNameLookup(), which would match TSHttpHdrReasonLookup().
>
> What values would TSHttpActionNameGet() accept?
>
> J
I prefer function names myself too. I prefer using *Get() since this is
used a lot more in the APIs vs *Lookup(). We should use Get as a common
action for APIs for consistency reasons.
-Bryan
Re: [API Review] Expose HttpDebugNames:: to plugins
Posted by Bryan Call <bc...@yahoo-inc.com>.
On Mar 1, 2014, at 9:01 PM, James Peach <jp...@apache.org> wrote:
> On Feb 28, 2014, at 3:21 PM, Leif Hedstrom <zw...@apache.org> wrote:
>
>> The internals of ATS has some static functions to convert from internal enum values to a descriptive string. This includes:
>>
>> HttpDebugNames::get_server_state_name()
>> HttpDebugNames::get_event_name()
>> HttpDebugNames::get_action_name()
>> HttpDebugNames::get_cache_action_name()
>> HttpDebugNames::get_api_hook_name()
>>
>>
>> I would like to expose these (and any future such Debug functions) to plugin APIs. A new one that would come to mind is a “id to overridable configuration” function (but, that’s for later). I think there’s at least two ways to do this, the first would be e.g.
>>
>> typedef enum
>> {
>> TS_HTTP_DEBUG_NAMES_NULL = -1,
>> TS_HTTP_DEBUG_NAMES_SERVER_STATE,
>> TS_HTTP_DEBUG_NAMES_EVENT_NAME,
>> TS_HTTP_DEBUG_NAMES_ACTION_NAME,
>> TS_HTTP_DEBUG_NAMES_CACHE_ACTION_NAME,
>> TS_HTTP_DEBUG_NAMES_API_HOOK_NAME,
>> TS_HTTP_DEBUG_NAMES_LAST_ENTRY
>> } TSHttpDebugNamesType;
>>
>> tsapi const char* TSHttpDebugNameGet(TSHttpDebugNamesType name, int value);
>>
>>
>> This has the advantage that it’s really easy to extend with future additions, easy to use / remember, and trivial to implement. The downside is there’s no type safety on the “value” parameter other than the fact that the underlying methods (see above) deals with them.
>>
>>
>> The other alternative is to add one C function for each type, e.g.
>>
>> tsapi const char* TSHttpServerStateNameGet(TSServerState value);
>> tsapi const char* TSHttpEventNameGet(TSEvent value);
>> tsapi const char* TSHttpActionNameGet(int value);
>>
>>
>> This has better type safety (in most cases), but makes it slightly harder to use and extend.
>>
>>
>> Please discuss which would be preferable. There’s currently no compatibility concerns regarding this addition, it’s a brand spanking new API. But we should design it for future compatibility.
>
> I prefer the function-based API; it's simpler to describe and use. I don't mind the suggested naming convention, but consider TSHttpServerStateNameLookup(), which would match TSHttpHdrReasonLookup().
>
> What values would TSHttpActionNameGet() accept?
>
> J
I prefer function names myself too. I prefer using *Get() since this is used a lot more in the APIs vs *Lookup(). We should use Get as a common action for APIs for consistency reasons.
-Bryan
Re: [API Review] Expose HttpDebugNames:: to plugins
Posted by James Peach <jp...@apache.org>.
On Feb 28, 2014, at 3:21 PM, Leif Hedstrom <zw...@apache.org> wrote:
> The internals of ATS has some static functions to convert from internal enum values to a descriptive string. This includes:
>
> HttpDebugNames::get_server_state_name()
> HttpDebugNames::get_event_name()
> HttpDebugNames::get_action_name()
> HttpDebugNames::get_cache_action_name()
> HttpDebugNames::get_api_hook_name()
>
>
> I would like to expose these (and any future such Debug functions) to plugin APIs. A new one that would come to mind is a “id to overridable configuration” function (but, that’s for later). I think there’s at least two ways to do this, the first would be e.g.
>
> typedef enum
> {
> TS_HTTP_DEBUG_NAMES_NULL = -1,
> TS_HTTP_DEBUG_NAMES_SERVER_STATE,
> TS_HTTP_DEBUG_NAMES_EVENT_NAME,
> TS_HTTP_DEBUG_NAMES_ACTION_NAME,
> TS_HTTP_DEBUG_NAMES_CACHE_ACTION_NAME,
> TS_HTTP_DEBUG_NAMES_API_HOOK_NAME,
> TS_HTTP_DEBUG_NAMES_LAST_ENTRY
> } TSHttpDebugNamesType;
>
> tsapi const char* TSHttpDebugNameGet(TSHttpDebugNamesType name, int value);
>
>
> This has the advantage that it’s really easy to extend with future additions, easy to use / remember, and trivial to implement. The downside is there’s no type safety on the “value” parameter other than the fact that the underlying methods (see above) deals with them.
>
>
> The other alternative is to add one C function for each type, e.g.
>
> tsapi const char* TSHttpServerStateNameGet(TSServerState value);
> tsapi const char* TSHttpEventNameGet(TSEvent value);
> tsapi const char* TSHttpActionNameGet(int value);
>
>
> This has better type safety (in most cases), but makes it slightly harder to use and extend.
>
>
> Please discuss which would be preferable. There’s currently no compatibility concerns regarding this addition, it’s a brand spanking new API. But we should design it for future compatibility.
I prefer the function-based API; it's simpler to describe and use. I don't mind the suggested naming convention, but consider TSHttpServerStateNameLookup(), which would match TSHttpHdrReasonLookup().
What values would TSHttpActionNameGet() accept?
J