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

Add default value for traceID header #246

Merged
merged 4 commits into from
Feb 19, 2022
Merged
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
6 changes: 1 addition & 5 deletions Sources/AWSLambdaRuntimeCore/ControlPlaneRequest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,10 @@ struct Invocation: Hashable {
throw Lambda.RuntimeError.invocationMissingHeader(AmazonHeaders.invokedFunctionARN)
}

guard let traceID = headers.first(name: AmazonHeaders.traceID) else {
throw Lambda.RuntimeError.invocationMissingHeader(AmazonHeaders.traceID)
}

self.requestID = requestID
self.deadlineInMillisSinceEpoch = unixTimeInMilliseconds
self.invokedFunctionARN = invokedFunctionARN
self.traceID = traceID
self.traceID = headers.first(name: AmazonHeaders.traceID) ?? "Root=\(AmazonHeaders.generateXRayTraceID());Sampled=0"
Copy link
Contributor

Choose a reason for hiding this comment

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

should we add a small unit test too so this does not regress?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As in a test checking the format of the generated traceID?

Copy link
Contributor

Choose a reason for hiding this comment

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

testing that we generate a default traceID if no header is provided

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have that ready. I used the header from another file and changed the year to 2022, but now soundness.sh is complaining that it's not an acceptable year. Should I add s/2022/YEARS/ to the replace_acceptable_years() function?

Copy link
Contributor

Choose a reason for hiding this comment

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

Should I add s/2022/YEARS/ to the replace_acceptable_years() function?

yes. sorry about the hassle!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See 374dc71

self.clientContext = headers["Lambda-Runtime-Client-Context"].first
self.cognitoIdentity = headers["Lambda-Runtime-Cognito-Identity"].first
}
Expand Down
37 changes: 37 additions & 0 deletions Tests/AWSLambdaRuntimeCoreTests/ControlPlaneRequestTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftAWSLambdaRuntime open source project
//
// Copyright (c) 2022 Apple Inc. and the SwiftAWSLambdaRuntime project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftAWSLambdaRuntime project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

@testable import AWSLambdaRuntimeCore
import NIOHTTP1
import XCTest

class InvocationTest: XCTestCase {
func testInvocationTraceID() throws {
let headers = HTTPHeaders([
(AmazonHeaders.requestID, "test"),
(AmazonHeaders.deadline, String(Date(timeIntervalSinceNow: 60).millisSinceEpoch)),
(AmazonHeaders.invokedFunctionARN, "arn:aws:lambda:us-east-1:123456789012:function:custom-runtime"),
])

var invocation: Invocation?

XCTAssertNoThrow(invocation = try Invocation(headers: headers))
XCTAssertNotNil(invocation)

guard !invocation!.traceID.isEmpty else {
XCTFail("Invocation traceID is empty")
return
}
}
}
2 changes: 1 addition & 1 deletion scripts/soundness.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ here="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"

function replace_acceptable_years() {
# this needs to replace all acceptable forms with 'YEARS'
sed -e 's/2017-2018/YEARS/' -e 's/2017-2020/YEARS/' -e 's/2017-2021/YEARS/' -e 's/2020-2021/YEARS/' -e 's/2019/YEARS/' -e 's/2020/YEARS/' -e 's/2021/YEARS/'
sed -e 's/2017-2018/YEARS/' -e 's/2017-2020/YEARS/' -e 's/2017-2021/YEARS/' -e 's/2020-2021/YEARS/' -e 's/2019/YEARS/' -e 's/2020/YEARS/' -e 's/2021/YEARS/' -e 's/2022/YEARS/'
}

printf "=> Checking for unacceptable language... "
Expand Down