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

Update data capture to match path pattern #60

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions ENV_VARS.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,7 @@ Agents can be configured using environment variables:
| HT_REPORTING_OPA_ENDPOINT | Represents the endpoint for polling OPA config file e.g. http://opa.traceableai:8181/ |
| HT_REPORTING_OPA_POLL_PERIOD_SECONDS | Poll period in seconds to query OPA service |
| HT_REPORTING_OPA_ENABLED | When `true` Open Policy Agent evaluation is enabled to block request |
| HT_DATA_CAPTURE_HTTP_HEADERS_REQUEST | When `false` it disables the capture for the request in a client/request operation |
| HT_DATA_CAPTURE_HTTP_HEADERS_RESPONSE | When `false` it disables the capture for the response in a client/request operation |
| HT_DATA_CAPTURE_HTTP_BODY_REQUEST | When `false` it disables the capture for the request in a client/request operation |
| HT_DATA_CAPTURE_HTTP_BODY_RESPONSE | When `false` it disables the capture for the response in a client/request operation |
| HT_DATA_CAPTURE_RPC_METADATA_REQUEST | When `false` it disables the capture for the request in a client/request operation |
| HT_DATA_CAPTURE_RPC_METADATA_RESPONSE | When `false` it disables the capture for the response in a client/request operation |
| HT_DATA_CAPTURE_RPC_BODY_REQUEST | When `false` it disables the capture for the request in a client/request operation |
| HT_DATA_CAPTURE_RPC_BODY_RESPONSE | When `false` it disables the capture for the response in a client/request operation |
| HT_DATA_CAPTURE_BODY_MAX_SIZE_BYTES | Maximum size of captured body in bytes. Default should be 131_072 (128 KiB). |
| HT_DATA_CAPTURE | Lists the rules for data being captured by instrumentation. The first rule in config that matches for a path should be used if multiple rules match. Therefore the default rule (if present) should be the last rule. If no default rule is given, the default behaviour should be to allow data capture for all elements. |
| HT_PROPAGATION_FORMATS | List the supported propagation formats e.g. `HT_PROPAGATION_FORMATS="B3,TRACECONTEXT"`. |
| HT_ENABLED | When `false`, disables the agent |
| HT_JAVAAGENT_FILTER_JAR_PATHS | Is the list of path to filter jars, separated by `,`. |
13 changes: 10 additions & 3 deletions config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,11 @@ message AgentConfig {
// reporting holds the reporting settings for the agent
Reporting reporting = 2;

// data_capture describes the data being captured by instrumentation
DataCapture data_capture = 3;
// data_capture lists the rules for data being captured by instrumentation.
// The first rule in config that matches for a path should be used if multiple rules match.
// Therefore the default rule (if present) should be the last rule.
// If no default rule is given, the default behaviour should be to allow data capture for all elements
repeated DataCapture data_capture = 3;
Copy link
Member

@pavolloffay pavolloffay Apr 9, 2021

Choose a reason for hiding this comment

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

I have mixed feelings from the above description. It's not vague per se, but not elegant either.

I would keep the original data_capture as the default one and add a new field e.g. endpoint_data_capture of a new type

message EndpointDataCapture{
   DataCapture data_capture = 1;
  // endpoint 
  // host 
  // probably also the sampling config 
}

Copy link
Member

Choose a reason for hiding this comment

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

We will also need whitelisted headers in the config https://github.com/hypertrace/specification/blob/main/agent/tracestate.md#whitelisted-headers. This should probably go to DataCapture

Copy link

@ryanericson ryanericson Apr 12, 2021

Choose a reason for hiding this comment

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

Let's take allowlisting headers separately, for the purpose of core mode processing it's even ok for it to be not configurable at the moment. I've opened https://traceableai.atlassian.net/browse/ENG-8716 to track it though.


// propagation_formats list the supported propagation formats
repeated PropagationFormat propagation_formats = 4;
Expand Down Expand Up @@ -78,7 +81,7 @@ message Message {
google.protobuf.BoolValue response = 2;
}

// DataCapture describes the elements to be captured by the agent instrumentation
// DataCapture describes the elements to be captured by the agent instrumentation for paths that match this rule.
message DataCapture {
// http_headers enables/disables the capture of the request/response headers in HTTP
Message http_headers = 1;
Expand All @@ -94,6 +97,10 @@ message DataCapture {

// maximum size of captured body in bytes. Default should be 131_072 (128 KiB).
google.protobuf.Int32Value body_max_size_bytes = 5;

// path_pattern is the path to match for this data capture rule.
// If path_pattern is not given, this rule will apply to all paths as the default
google.protobuf.StringValue path_pattern = 6;
}

// PropagationFormat represents the propagation formats supported by agents
Expand Down