Repository navigation
DPS-45438: ACLP Logs - add new TrafficPeak destination type - #1044
mduda-akamai wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
🟡 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_byvalue 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
CreateLogsDestinationfails, the followingrequire.NoErroraborts the test beforefixtureTeardownis called. In recording mode this leaves the go-vcr recorder open and can leave the cassette incomplete; the existingsetupLogsDestinationpath explicitly closes its recorder on creation failure. ClosefixtureTeardownon 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
stringvalues withjson:",omitzero"inLogsDestinationCustomHTTPSDetailsUpdateOptions. That conflicts with the repository rule inAGENTS.md:40-41, which requires everyomitzerofield 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.
22661ac to
92c1999
Compare
92c1999 to
9f532be
Compare
There was a problem hiding this comment.
🟡 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
📝 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
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.