Skip to content

feat: Allow for custom endpoints in chronicleexporter/http_exporter #2167

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: Allow for custom endpoints in chronicleexporter/http_exporter #2167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions exporter/chronicleexporter/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ func (cfg *Config) Validate() error {
}

if cfg.Protocol == protocolHTTPS {
if cfg.Location == "" {
return errors.New("location is required when protocol is https")
Comment on lines -140 to -141
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is still required because this parameter is used for every log that's sent in the constructed forwarder string. We'll probably need a different way/new parameter to indicate a custom endpoint. https://github.com/observIQ/bindplane-otel-collector/blob/main/exporter/chronicleexporter/marshal.go#L532-L535

if cfg.Endpoint == "" {
return errors.New("endpoint is required when protocol is https")
}
if cfg.Project == "" {
return errors.New("project is required when protocol is https")
Expand Down
9 changes: 7 additions & 2 deletions exporter/chronicleexporter/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func TestConfigValidate(t *testing.T) {
expectedErr: "invalid compression type",
},
{
desc: "Protocol is https and location is empty",
desc: "Protocol is https and endpoint is empty",
config: &Config{
CredsFilePath: "/path/to/creds_file",
LogType: "log_type_example",
Expand All @@ -120,11 +120,12 @@ func TestConfigValidate(t *testing.T) {
BatchRequestSizeLimitHTTP: defaultBatchRequestSizeLimitHTTP,
BatchLogCountLimitHTTP: defaultBatchLogCountLimitHTTP,
},
expectedErr: "location is required when protocol is https",
expectedErr: "endpoint is required when protocol is https",
},
{
desc: "Protocol is https and forwarder is empty",
config: &Config{
Endpoint: "myendpoint.com",
CredsFilePath: "/path/to/creds_file",
LogType: "log_type_example",
Protocol: protocolHTTPS,
Expand All @@ -139,6 +140,7 @@ func TestConfigValidate(t *testing.T) {
{
desc: "Protocol is https and project is empty",
config: &Config{
Endpoint: "myendpoint.com",
CredsFilePath: "/path/to/creds_file",
LogType: "log_type_example",
Protocol: protocolHTTPS,
Expand All @@ -153,6 +155,7 @@ func TestConfigValidate(t *testing.T) {
{
desc: "Protocol is https and http batch log count limit is 0",
config: &Config{
Endpoint: "myendpoint.com",
CredsFilePath: "/path/to/creds_file",
LogType: "log_type_example",
Protocol: protocolHTTPS,
Expand All @@ -168,6 +171,7 @@ func TestConfigValidate(t *testing.T) {
{
desc: "Protocol is https and http batch request size limit is 0",
config: &Config{
Endpoint: "myendpoint.com",
CredsFilePath: "/path/to/creds_file",
LogType: "log_type_example",
Protocol: protocolHTTPS,
Expand All @@ -183,6 +187,7 @@ func TestConfigValidate(t *testing.T) {
{
desc: "Valid https config",
config: &Config{
Endpoint: "myendpoint.com",
CredsFilePath: "/path/to/creds_file",
LogType: "log_type_example",
Protocol: protocolHTTPS,
Expand Down
3 changes: 3 additions & 0 deletions exporter/chronicleexporter/http_exporter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@ func (exp *httpExporter) uploadToChronicleHTTP(ctx context.Context, logs *api.Im
// URL for the request: https://{region}-chronicle.googleapis.com/{version}/projects/{project}/location/{region}/instances/{customerID}
// Override for testing
var httpEndpoint = func(cfg *Config, logType string) string {
if cfg.Location == "" {
return fmt.Sprintf("https://%s/v1alpha/projects/%s/instances/%s/logTypes/%s/logs:import", cfg.Endpoint, cfg.Project, cfg.CustomerID, logType)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still require the location value in the URL? Asked Travis about this and he was going to test it out.

}
formatString := "https://%s-%s/v1alpha/projects/%s/locations/%s/instances/%s/logTypes/%s/logs:import"
return fmt.Sprintf(formatString, cfg.Location, cfg.Endpoint, cfg.Project, cfg.Location, cfg.CustomerID, logType)
}
Loading