Skip to content

DPS-45438: ACLP Logs - add new TrafficPeak destination type - #1044

Open
mduda-akamai wants to merge 3 commits into
linode:mainfrom
mduda-akamai:feature/DPS-45125-ACLP-Logs-add-new-TrafficPeak-destination-type
Open

mduda-akamai wants to merge 3 commits into
linode:mainfrom
mduda-akamai:feature/DPS-45125-ACLP-Logs-add-new-TrafficPeak-destination-type

Conversation

@mduda-akamai

Copy link
Copy Markdown

📝 Description

What does this PR do and why is this change necessary?
Add support for a new destination type: TrafficPeak in ACLP Logs, add missing integration tests for Custom HTTPS destination

Note: The TrafficPeak destination is not yet supported in Linode's APIv4. I'll merge this PR once it's implemented there.

✔️ How to Test

What are the steps to reproduce the issue or verify the changes?
https://techdocs.akamai.com/linode-api/reference/post-destination

How do I run the relevant unit/integration tests?
Follow the usual installation/setup steps

  • Integration tests: make test-int
    Note: The TrafficPeak destination is not yet supported in Linode's APIv4, so the TrafficPeak integration tests are disabled for now with the RUN_TRAFFIC_PEAK_DESTINATION_TESTS flag by default.
  • Unit tests: make test-unit

Copilot AI lite review requested due to automatic review settings September 14, 2026 12:54
@mduda-akamai
mduda-akamai requested review from a team as code owners September 14, 2026 12:54
@mduda-akamai
mduda-akamai requested review from mawilk90 and zliang-akamai and removed request for a team September 14, 2026 12:54
@mduda-akamai mduda-akamai changed the title DPS-45125: ACLP Logs - add new TrafficPeak destination type DPS-45438: ACLP Logs - add new TrafficPeak destination type Sep 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Fixture sanitization, replay-mode coverage, and recorder cleanup issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds TrafficPeak as an ACLP Logs destination and expands Custom HTTPS integration coverage.

Changes:

  • Adds TrafficPeak models, constants, fixtures, and serialization tests.
  • Adds Custom HTTPS CRUD, history, and stream integration tests.
  • Adds VCR fixtures for the new integration coverage.
File summaries
File Reviewed change Findings
test/unit/monitor_logs_test.go TrafficPeak serialization and response tests Nit: Use pointer strings for omitzero options and assert omission with nil (1 vote).
test/unit/fixtures/monitor_log_stream_traffic_peak.json TrafficPeak stream fixture No final comments.
test/unit/fixtures/monitor_log_destination_traffic_peak.json TrafficPeak destination fixture No final comments.
test/integration/monitor_logs_test.go TrafficPeak and Custom HTTPS integration tests Moderate: Enable Custom HTTPS tests in replay mode (3 votes); close fixture teardown when destination creation fails (1 vote).
test/integration/fixtures/TestLogStream_CustomHTTPSDestination.yaml Custom HTTPS stream cassette Moderate: Sanitize repeated updated_by identifiers (1 vote).
test/integration/fixtures/TestLogsDestination_CustomHTTPSUpdateAndHistory.yaml Custom HTTPS update/history cassette No final comments.
test/integration/fixtures/TestLogsDestination_CustomHTTPSList.yaml Custom HTTPS list cassette Moderate: Sanitize or reduce unrelated endpoint and certificate data (1 vote).
test/integration/fixtures/TestLogsDestination_CustomHTTPSGet.yaml Custom HTTPS get cassette No final comments.
test/integration/fixtures/TestLogsDestination_CustomHTTPSDelete.yaml Custom HTTPS delete cassette No final comments.
test/integration/fixtures/TestLogsDestination_CustomHTTPSCreate.yaml Custom HTTPS create cassette No final comments.
monitor_log_streams.go TrafficPeak stream destination constant No final comments.
monitor_log_destinations.go TrafficPeak destination models and options No final comments.
Review details

Suppressed comments (4)

test/integration/fixtures/TestLogStream_CustomHTTPSDestination.yaml:89

  • This new cassette leaves the destination's internal updated_by value unsanitized in the stream responses. Since the fixture is committed and the recorder only scrubs selected fields, replace this account identifier (and its repeated occurrences) with a sanitized value before merging.
      "created_by": "[SANITIZED]", "created": "2018-01-02T03:04:05", "updated_by":
      "mduda_aclplogs_dev", "updated": "2018-01-02T03:04:05"}'

test/integration/fixtures/TestLogsDestination_CustomHTTPSList.yaml:169

  • This newly added list cassette contains an unrelated live custom-HTTPS destination with an internal endpoint and full client/CA certificates. The fixture sanitizer does not redact these fields, so committing this recording exposes infrastructure data and adds large unrelated state to a test that only needs the created destination; please sanitize or reduce the response before merging.
      "details": {"endpoint_url": "https://pl-labkrk-2-https-server-dev.cloud-streams-dev.akadns.net/stream-custom-https-server-api/logs/basic-auth",
      "authentication": {"type": "basic"}, "data_compression": "gzip", "client_certificate_details":
      {"client_ca_certificate": "-----BEGIN CERTIFICATE-----\nMIIFmjCCA4KgAwIBAgIUdjTxaJKvNQpAFMvZ3EZibl9AiXQwDQYJKoZIhvcNAQEL\nBQAwgbYxCzAJBgNVBAYTAlVTMRYwFAYDVQQIDA1NYXNzYWNodXNldHRzMRIwEAYD\nVQQHDAlDYW1icmlkZ2UxHDAaBgNVBAoME0FrYW1haSBUZWNobm9sb2dpZXMxDDAK\nBgNVBAsMA0tNSTEwMC4GA1UEAwwnYWthbWFpLmNsb3VkX2xvZ3NfY2EuY29uZmln\nX3NlcnZlcl9jYS42MR0wGwYJKoZIhvcNAQkBFg5rbWlAYWthbWFpLmNvbTAeFw0y\nNjA3MjIxNDE4MTRaFw0yNjA4MDUxNDE4MTRaMIGtMQswCQYDVQQGEwJVUzEWMBQG\nA1UECAwNTWFzc2FjaHVzZXR0czESMBAGA1UEBwwJQ2FtYnJpZGdlMRwwGgYDVQQK\nDBNBa2FtYWkgVGVjaG5vbG9naWVzMQwwCgYDVQQLDANLTUkxJzAlBgNVBAMMHiou\nY2xvdWQtc3RyZWFtcy1kZXYuYWthZG5zLm5ldDEdMBsGCSqGSIb3DQEJARYOa21p\nQGFrYW1haS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCdHcm1\nZE8wboOtaHhmGwpUciYg5vm8wYr2GHyK19DEFAjzTX/k9TWq61SMtNkbEtEGdFRB\nUXrA6bErGgsJv7JSefQug228R2cJepsKFntQhUHm2pJuISR6/emxln1RcO4/6irt\n+SRhKeoLHUE/cB+QlUuNbZ3JEZTvkmR26lyoqhO8xvLMI2RwAnxfjMpY9g4fgOBb\n8YyY6KBVCtVAbLnLAHmagjx+8mVXmO8WQ7fu1DMhweTGZVjgXk6A/Ak29gFdmNsu\nB3hnOi+YAJnbaVVSgDqU7yQ4REJ/Wntif/GtYq5qp2NVeQ6MJ6QvJPha+znVnWnK\nBvs2WOxoCx7xfvCDAgMBAAGjgaYwgaMwDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQU\nNtT4a269EAFJ7/kPPvlXMHB8dNkwUwYDVR0RAQH/BEkwR4IeKi5jbG91ZC1zdHJl\nYW1zLWRldi5ha2FkbnMubmV0giUqLmFjbHBsb2dzZGV2Y2xvdWQuYXJtYWRhLmFr\nYXBsYXQubmV0MB8GA1UdIwQYMBaAFBulJiYHVKia7JOdkV84fsh7X0XUMA0GCSqG\nSIb3DQEBCwUAA4ICAQBv+HC8OZ2wHv/zTE1bJrxQr/gAcIV4hhJ3Dg8jpJ6e2TBI\nY0bR354ThDUD6dcP0/6xUppR7UY6mmOmlmg7CV33BwC2MnONxbdq64GasVnBbP6f\nTrlLteA6AR0MG1CWThnkNWb0n7EONDANQIbFGDO5eTcjHm0aTySnfjvqwUG6b+X9\n5kkmPFWI5R1TGOi4uFFnu/7mog3GGLCqOm2UHbQBQOanb5fkouobWlxx21ahsrAL\nZmgNA8mYq/+G8tS/obrxt8OzRp+RTaillAoTA8Div08tnmY7sdVdnj6EuvjprEIb\n1H+cW5GULnszXtxRbBoRlO6MM+IKWpzfuF7xU3d4DkPxUI9ouA6hDJFBMMFOyUpf\neYPGfybchzKBnD5TKg3pFWSBE8yWsik2H6zngFONZFtpDglkU06ZNHyZDWExRJZd\nHHy5EwFI7w4PIeQ/KAcod+TKQdsyG3nFZ77Unl25O9Ub9sC1D0fNsE35Fsu3R8i6\n/VgEu+ss3moSpdlxZUv2Y4SNZ07h2qnqQK0J9UwI8bIzLHxIAEfS/XzxKLwQ2iHt\nRKK3mwHFs4dLHZ9rwAqUDxnaffwIYXdSnIbSdFs7NTeLSgHu8HjJ+xpTIkgYWtV9\npVIWhqNEyVfnV/oBD9yBRz7DrswveWRsOfOVVyakfBWGNJsAzqOiJ5yMyt5Iqg==\n-----END
      CERTIFICATE-----\n", "client_certificate": "-----BEGIN CERTIFICATE-----\nMIIFOjCCAyKgAwIBAgIUcEe6I2y8mZ2HpVl8bwi0NWYdZFYwDQYJKoZIhvcNAQEL\nBQAwgbYxCzAJBgNVBAYTAlVTMRYwFAYDVQQIDA1NYXNzYWNodXNldHRzMRIwEAYD\nVQQHDAlDYW1icmlkZ2UxHDAaBgNVBAoME0FrYW1haSBUZWNobm9sb2dpZXMxDDAK\nBgNVBAsMA0tNSTEwMC4GA1UEAwwnYWthbWFpLmNsb3VkX2xvZ3NfY2EuY29uZmln\nX2NsaWVudF9jYS4zMR0wGwYJKoZIhvcNAQkBFg5rbWlAYWthbWFpLmNvbTAeFw0y\nNjAzMTgxMzMwNTBaFw0yNjA0MDExMzMwNTBaMIGkMQswCQYDVQQGEwJVUzEWMBQG\nA1UECAwNTWFzc2FjaHVzZXR0czESMBAGA1UEBwwJQ2FtYnJpZGdlMRwwGgYDVQQK\nDBNBa2FtYWkgVGVjaG5vbG9naWVzMQwwCgYDVQQLDANLTUkxHjAcBgNVBAMMFWNy\nb24tam9iLTAuY2xvdWQtbG9nczEdMBsGCSqGSIb3DQEJARYOa21pQGFrYW1haS5j\nb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCmj+X/8IpJnCZhpt2b\n7ppP7Xmn22dpB8uo9MilECExDaQTtFM9E9JqZP+miSL4OkTefOv//LGFKVuY6Q+J\nensAIMbEa2oJpgak1gAOQxxrwGdGnQi4smtf9EHQqELmPDZhPNoxWvYKUrGYfNvx\n3UcU+hvO6OO5ZSsgyT5I7UJM2WcmV8YeWBPLvq7X8hfVbZrWIIpmaaPAA1QKKOJi\nH0J+1NTIYMhzIrsCJUdpcootETWk+TeGndxiMGe4nFBJ4ND4EV5r2pBlkxCB8Ssq\nYymZaf9rtjSmk/YmS69UMJDu1UIiqz4Tx1yXYiaTuqG5m84JejfkaSvbxWrUIIeW\nWXEXAgMBAAGjUDBOMAwGA1UdEwEB/wQCMAAwHQYDVR0OBBYEFLGGqz9jakOoI6Y0\nRAygaYSFVI+SMB8GA1UdIwQYMBaAFIhIuE7G0kuAGvolB1LwPCRK/+UuMA0GCSqG\nSIb3DQEBCwUAA4ICAQBUyeD5n5uL3P32PFnQrugbpBGTs6fzwQ7wCbZCqOi2h9BU\nflm3qyxLL1exWi3VWhJ48Cesvcl7C47fZn3qFWIJLUnmcBOYpuoDQRx371XQWnBZ\nXsf5F08Fj6yAmWhlV3S7j8zoxJggckvoCkD/AcAspIF2FABelWBjrvCufxGM6NBJ\njDlgyT+of6NCps/rTGIfpQEfo7qjZdNRZmMaDzM9JPUFDA0Pgm5Z7d/SNZFWqljJ\nV7Y20LcV4qjZu50NYdN9bNauOb/QI44BCSoRuJ0xV9Mw69n9Xpfes/EMkT09IJDP\nNKmVgnVCl0xvS4w7+6/+EgLQfrTbDTdZc7jU9OX02yfGzfRq8ZOPNC+ir452Leoo\nogJWpdBjTIs0XQjIkFCxMT21twFTCnjsIQ89IOEZGHsPCc53UyevjJPKs6QT+nAP\nkYRueIVjEnJV5izAqUwEk/uwROqwSC9vZwWl/6niKHYY9jybRLBsnMv7vs3gqUmf\ndjrAzS6LQkMdBqfZYvpHABH+xyllvvaJNkcj4+Vs1ujXcRT/BNnEklizTIkdMY0+\ncOlyHEmpSurtlSfHTbDk9N+4BsIpq0IpmFGGP3W8IBuiRkvBpZc6ah31X86exRva\nR0jdZX8jzgchghcNaUuxg/agg3zScd4ag4flxM24cCq4lmR963b7yQjxI4HIwA==\n-----END
      CERTIFICATE-----\n"}, "custom_headers": [{"name": "X-Logs-Custom-Path", "value":

test/integration/monitor_logs_test.go:201

  • If CreateLogsDestination fails, the following require.NoError aborts the test before fixtureTeardown is called. In recording mode this leaves the go-vcr recorder open and can leave the cassette incomplete; the existing setupLogsDestination path explicitly closes its recorder on creation failure. Close fixtureTeardown on this error path before failing the test.
	destination, err := client.CreateLogsDestination(context.Background(), linodego.LogsDestinationCreateOptions{
		Label:   testLabel(),
		Type:    destinationType,
		Details: details,
	})

test/unit/monitor_logs_test.go:352

  • This test codifies empty-string omission for fields that are declared as plain string values with json:",omitzero" in LogsDestinationCustomHTTPSDetailsUpdateOptions. That conflicts with the repository rule in AGENTS.md:40-41, which requires every omitzero field in option structs to be nil-able so omitted and explicit zero values remain distinguishable. Change those options to pointer strings and assert omission with nil instead of preserving this test.
		Details: &linodego.LogsDestinationCustomHTTPSDetailsUpdateOptions{
			EndpointURL:     "",
			ContentType:     "",
			DataCompression: "",
		},
  • Files reviewed: 12/12 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/integration/monitor_logs_test.go
@mduda-akamai
mduda-akamai force-pushed the feature/DPS-45125-ACLP-Logs-add-new-TrafficPeak-destination-type branch from 22661ac to 92c1999 Compare September 14, 2026 13:00
@mduda-akamai
mduda-akamai force-pushed the feature/DPS-45125-ACLP-Logs-add-new-TrafficPeak-destination-type branch from 92c1999 to 9f532be Compare September 14, 2026 13:45
@yec-akamai yec-akamai added the community-contribution contributions from the community. label Sep 14, 2026
@yec-akamai
yec-akamai requested a balanced review from Copilot September 16, 2026 20:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The TrafficPeak header request type is incompatible with the corresponding response field type.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 13/13 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread monitor_log_destinations.go Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution contributions from the community.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants