Skip to content

Add a set-http-cntl operator for header_rewrite - #8375

Merged
zwoop merged 1 commit into
apache:masterfrom
zwoop:HWLoggingOperator
Nov 17, 2021
Merged

Add a set-http-cntl operator for header_rewrite#8375
zwoop merged 1 commit into
apache:masterfrom
zwoop:HWLoggingOperator

Conversation

@zwoop

@zwoop zwoop commented Oct 4, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

This supports all the controllers that the new InkAPI supports, unclear if all or any of them are useful here. But the LOGGING controller definitely is.

        LOGGING          TS_HTTP_CNTL_LOGGING_MODE
        INTERCEPT_RETRY  TS_HTTP_CNTL_INTERCEPT_RETRY_MODE
        RESP_CACHEABLE   TS_HTTP_CNTL_RESPONSE_CACHEABLE
        REQ_CACHEABLE    TS_HTTP_CNTL_REQUEST_CACHEABLE
        SERVER_NO_STORE  TS_HTTP_CNTL_SERVER_NO_STORE
        TXN_DEBUG        TS_HTTP_CNTL_TXN_DEBUG
        SKIP_REMAP       TS_HTTP_CNTL_SKIP_REMAPPING

@zwoop zwoop added this to the 10.0.0 milestone Oct 4, 2021
@zwoop zwoop self-assigned this Oct 4, 2021
@zwoop
zwoop requested a review from bryancall as a code owner October 4, 2021 21:48
@zwoop

zwoop commented Oct 4, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Example usage could be something like this, for a 10% log sampling rate:

cond %{REMAP_PSEUDO_HOOK}
cond %{RANDOM:100} >10
  set-http-cntl LOGGING FALSE

@bryancall
bryancall requested a review from ywkaras October 4, 2021 23:05
@zwoop

zwoop commented Oct 4, 2021

Copy link
Copy Markdown
Contributor Author

Here's a list of existing APIs that could be moved into this generic one:

tsapi void TSHttpTxnRespCacheableSet(TSHttpTxn txnp, int flag);
tsapi void TSHttpTxnReqCacheableSet(TSHttpTxn txnp, int flag);
tsapi TSReturnCode TSHttpTxnServerRespNoStoreSet(TSHttpTxn txnp, int flag);
tsapi void TSHttpTxnDebugSet(TSHttpTxn txnp, int on);
tsapi void TSHttpSsnDebugSet(TSHttpSsn ssnp, int on);
tsapi void TSSkipRemappingSet(TSHttpTxn txnp, int flag);

@ywkaras

ywkaras commented Oct 5, 2021

Copy link
Copy Markdown

Would it be hard to added testing for this to pluginTest/header_rewrite/header_rewrite.test.py or add a new test?

@zwoop

zwoop commented Oct 5, 2021

Copy link
Copy Markdown
Contributor Author

Would it be hard to added testing for this to pluginTest/header_rewrite/header_rewrite.test.py or add a new test?

Dunno, I think so? It'd have to setup logging, as well as verifying that the logging is happening or not happening? Just take my word for it, I mean, if I were wrong (in the code) don't you think I would know it?

ywkaras
ywkaras previously approved these changes Oct 5, 2021
View reviewed changes

@ywkaras ywkaras left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's all about trust, not testing. We must all swim naked together in volcanic hot springs in Iceland ASAP to build trust.

@bryancall

bryancall commented Oct 14, 2021

Copy link
Copy Markdown
Contributor

[approve ci]

1 similar comment
@bryancall

bryancall commented Oct 14, 2021

Copy link
Copy Markdown
Contributor

[approve ci]

@zwoop
zwoop force-pushed the HWLoggingOperator branch from f3479ab to 65ae442 Compare October 25, 2021 14:11
@bneradt

bneradt commented Oct 25, 2021

Copy link
Copy Markdown
Contributor

[approve ci clang-analyzer]

@zwoop
zwoop force-pushed the HWLoggingOperator branch from 65ae442 to e4a9d26 Compare October 30, 2021 03:42
@zwoop zwoop changed the title Add a set-logging operator for header_rewrite Add a set-http-cntl operator for header_rewrite Oct 30, 2021
@zwoop
zwoop force-pushed the HWLoggingOperator branch 2 times, most recently from e28e69c to fe80d29 Compare October 30, 2021 03:59
@zwoop

zwoop commented Nov 1, 2021

Copy link
Copy Markdown
Contributor Author

I rewrote / updated this to be more generic, now that we landed the new API as well.

@zwoop
zwoop force-pushed the HWLoggingOperator branch from fe80d29 to 472fe88 Compare November 1, 2021 16:05
@zwoop
Adds the set-http-cntl operator for header_rewrite
d04b5ec 
This supports all the controllers that the InkAPI supports,
unclear if all or any of them are useful here. But the LOGGING
controller definitely is.

    LOGGING          TS_HTTP_CNTL_LOGGING_MODE
    INTERCEPT_RETRY  TS_HTTP_CNTL_INTERCEPT_RETRY_MODE
    RESP_CACHEABLE   TS_HTTP_CNTL_RESPONSE_CACHEABLE
    REQ_CACHEABLE    TS_HTTP_CNTL_REQUEST_CACHEABLE
    SERVER_NO_STORE  TS_HTTP_CNTL_SERVER_NO_STORE
    TXN_DEBUG        TS_HTTP_CNTL_TXN_DEBUG
    SKIP_REMAP       TS_HTTP_CNTL_SKIP_REMAPPING
@zwoop
zwoop force-pushed the HWLoggingOperator branch from 472fe88 to d04b5ec Compare November 1, 2021 16:07
@zwoop
zwoop requested a review from elsloo November 1, 2021 22:53
ywkaras
ywkaras reviewed Nov 9, 2021
View reviewed changes
Comment thread doc/admin-guide/plugins/header_rewrite.en.rst

set-http-cntl
~~~~~~~~~~~~~
;;

@ywkaras ywkaras Nov 9, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this some sphinx voodoo or are you just winking at us twice?

ywkaras
ywkaras approved these changes Nov 9, 2021
View reviewed changes
Comment thread plugins/header_rewrite/operators.h

private:
bool _flag = false;
TSHttpCntlType _cntl_qual;

@ywkaras ywkaras Nov 9, 2021

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm seems pointless to initialize one of these and not the other.

@zwoop
zwoop merged commit 4547ed7 into apache:master Nov 17, 2021
@zwoop
zwoop deleted the HWLoggingOperator branch November 17, 2021 17:38
zwoop added a commit that referenced this pull request Nov 17, 2021
@zwoop
Adds the set-http-cntl operator for header_rewrite (#8375)
746d022 
This supports all the controllers that the InkAPI supports,
unclear if all or any of them are useful here. But the LOGGING
controller definitely is.

    LOGGING          TS_HTTP_CNTL_LOGGING_MODE
    INTERCEPT_RETRY  TS_HTTP_CNTL_INTERCEPT_RETRY_MODE
    RESP_CACHEABLE   TS_HTTP_CNTL_RESPONSE_CACHEABLE
    REQ_CACHEABLE    TS_HTTP_CNTL_REQUEST_CACHEABLE
    SERVER_NO_STORE  TS_HTTP_CNTL_SERVER_NO_STORE
    TXN_DEBUG        TS_HTTP_CNTL_TXN_DEBUG
    SKIP_REMAP       TS_HTTP_CNTL_SKIP_REMAPPING

(cherry picked from commit 4547ed7)
@zwoop

zwoop commented Nov 17, 2021

Copy link
Copy Markdown
Contributor Author

Cherry-picked to v9.2.x

@zwoop zwoop modified the milestones: 10.0.0, 9.2.0 Nov 17, 2021
moonchen pushed a commit to moonchen/trafficserver that referenced this pull request Mar 17, 2022
@zwoop
Merge remote-tracking branch 'asf/9.2.x' into 9.2.x
2210491 
* asf/9.2.x:
  Updated ChangeLog
  Marks tls_engine test as OpenSSL-only (apache#8509)
  Add skipped log entries to troubleshooting FAQ in docs. (apache#8507)
  Remove commented out EventProcessor::eventthread - it's time (apache#8517)
  Makes proxy_serve_stale version independent (apache#8508)
  Add proxy.config.http2.stream_error_sampling_threshold (apache#8506)
  url_sig: change cleanup msg from TSError to TSDebug (apache#8523)
  Adds the set-http-cntl operator for header_rewrite (apache#8375)
  Prevent bad ssl_multicert.config load from being swapped in (apache#8515)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ywkaras ywkaras ywkaras approved these changes
@bryancall bryancall Awaiting requested review from bryancall
@elsloo elsloo Awaiting requested review from elsloo

Assignees

@zwoop zwoop

Labels

Plugins TS API

Projects

None yet

Milestone

9.2.0

Development

Successfully merging this pull request may close these issues.

5 participants

@zwoop @ywkaras @bryancall @bneradt @masaori335