diff --git a/awsplugins/beanstalk/beanstalk.go b/awsplugins/beanstalk/beanstalk.go index e153c065..5842f6b7 100644 --- a/awsplugins/beanstalk/beanstalk.go +++ b/awsplugins/beanstalk/beanstalk.go @@ -16,8 +16,10 @@ import ( "github.com/aws/aws-xray-sdk-go/internal/plugins" ) +// Origin is the type of AWS resource that runs your application. const Origin = "AWS::ElasticBeanstalk::Environment" +// Init activates ElasticBeanstalkPlugin at runtime. func Init() { if plugins.InstancePluginMetadata != nil && plugins.InstancePluginMetadata.BeanstalkMetadata == nil { addPluginMetadata(plugins.InstancePluginMetadata) diff --git a/awsplugins/ec2/ec2.go b/awsplugins/ec2/ec2.go index 6dbbe3e0..cb03adc8 100644 --- a/awsplugins/ec2/ec2.go +++ b/awsplugins/ec2/ec2.go @@ -15,8 +15,10 @@ import ( "github.com/aws/aws-xray-sdk-go/internal/plugins" ) +// Origin is the type of AWS resource that runs your application. const Origin = "AWS::EC2::Instance" +// Init activates EC2Plugin at runtime. func Init() { if plugins.InstancePluginMetadata != nil && plugins.InstancePluginMetadata.EC2Metadata == nil { addPluginMetadata(plugins.InstancePluginMetadata) diff --git a/awsplugins/ecs/ecs.go b/awsplugins/ecs/ecs.go index 954ee26a..bb29ff93 100644 --- a/awsplugins/ecs/ecs.go +++ b/awsplugins/ecs/ecs.go @@ -15,8 +15,10 @@ import ( "github.com/aws/aws-xray-sdk-go/internal/plugins" ) +// Origin is the type of AWS resource that runs your application. const Origin = "AWS::ECS::Container" +// Init activates ECSPlugin at runtime. func Init() { if plugins.InstancePluginMetadata != nil && plugins.InstancePluginMetadata.ECSMetadata == nil { addPluginMetadata(plugins.InstancePluginMetadata) diff --git a/daemoncfg/daemon_config.go b/daemoncfg/daemon_config.go index a5a91e4e..3425877d 100644 --- a/daemoncfg/daemon_config.go +++ b/daemoncfg/daemon_config.go @@ -5,6 +5,7 @@ // http://aws.amazon.com/apache2.0/ // // or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + package daemoncfg import ( @@ -21,12 +22,12 @@ var addressDelimiter = " " // delimiter between tcp and udp addresses var udpKey = "udp" var tcpKey = "tcp" -/// DaemonEndpoints stores X-Ray daemon configuration about the ip address and port for UDP and TCP port. It gets the address -/// string from "AWS_TRACING_DAEMON_ADDRESS" and then from recorder's configuration for DaemonAddr. -/// A notation of '127.0.0.1:2000' or 'tcp:127.0.0.1:2000 udp:127.0.0.2:2001' or 'udp:127.0.0.1:2000 tcp:127.0.0.2:2001' -/// are both acceptable. The first one means UDP and TCP are running at the same address. -/// Notation 'hostname:2000' or 'tcp:hostname:2000 udp:hostname:2001' or 'udp:hostname:2000 tcp:hostname:2001' are also acceptable. -/// By default it assumes a X-Ray daemon running at 127.0.0.1:2000 listening to both UDP and TCP traffic. +// DaemonEndpoints stores X-Ray daemon configuration about the ip address and port for UDP and TCP port. It gets the address +// string from "AWS_TRACING_DAEMON_ADDRESS" and then from recorder's configuration for DaemonAddr. +// A notation of '127.0.0.1:2000' or 'tcp:127.0.0.1:2000 udp:127.0.0.2:2001' or 'udp:127.0.0.1:2000 tcp:127.0.0.2:2001' +// are both acceptable. The first one means UDP and TCP are running at the same address. +// Notation 'hostname:2000' or 'tcp:hostname:2000 udp:hostname:2001' or 'udp:hostname:2000 tcp:hostname:2001' are also acceptable. +// By default it assumes a X-Ray daemon running at 127.0.0.1:2000 listening to both UDP and TCP traffic. type DaemonEndpoints struct { // UDPAddr represents UDP endpoint for segments to be sent by emitter. UDPAddr *net.UDPAddr @@ -93,13 +94,13 @@ func GetDaemonEndpointsFromString(dAddr string) (*DaemonEndpoints, error) { func resolveAddress(dAddr string) (*DaemonEndpoints, error) { addr := strings.Split(dAddr, addressDelimiter) - if len(addr) == 1 { + switch len(addr) { + case 1: return parseSingleForm(addr[0]) - } else if len(addr) == 2 { + case 2: return parseDoubleForm(addr) - } else { - return nil, errors.New("invalid daemon address: " + dAddr) } + return nil, errors.New("invalid daemon address: " + dAddr) } func parseDoubleForm(addr []string) (*DaemonEndpoints, error) { diff --git a/header/header.go b/header/header.go index 7d5f12b5..3b9441e6 100644 --- a/header/header.go +++ b/header/header.go @@ -54,11 +54,12 @@ const ( ) func samplingDecision(s string) SamplingDecision { - if s == string(Sampled) { + switch s { + case string(Sampled): return Sampled - } else if s == string(NotSampled) { + case string(NotSampled): return NotSampled - } else if s == string(Requested) { + case string(Requested): return Requested } return Unknown @@ -84,13 +85,14 @@ func FromString(s string) *Header { p := strings.TrimSpace(parts[i]) value, valid := valueFromKeyValuePair(p) if valid { - if strings.HasPrefix(p, RootPrefix) { + switch { + case strings.HasPrefix(p, RootPrefix): ret.TraceID = value - } else if strings.HasPrefix(p, ParentPrefix) { + case strings.HasPrefix(p, ParentPrefix): ret.ParentID = value - } else if strings.HasPrefix(p, SampledPrefix) { + case strings.HasPrefix(p, SampledPrefix): ret.SamplingDecision = samplingDecision(p) - } else if !strings.HasPrefix(p, SelfPrefix) { + case !strings.HasPrefix(p, SelfPrefix): key, valid := keyFromKeyValuePair(p) if valid { ret.AdditionalData[key] = value diff --git a/internal/logger/logger.go b/internal/logger/logger.go index 6ab83266..8e9a56ee 100644 --- a/internal/logger/logger.go +++ b/internal/logger/logger.go @@ -17,7 +17,7 @@ import ( // This internal package hides the actual logging functions from the user. -// The Logger instance used by xray to log. Set via xray.SetLogger(). +// Logger instance used by xray to log. Set via xray.SetLogger(). var Logger xraylog.Logger = xraylog.NewDefaultLogger(os.Stdout, xraylog.LogLevelInfo) func Debugf(format string, args ...interface{}) { diff --git a/internal/plugins/plugin.go b/internal/plugins/plugin.go index 636635d0..192061c7 100644 --- a/internal/plugins/plugin.go +++ b/internal/plugins/plugin.go @@ -9,8 +9,13 @@ package plugins const ( - EBServiceName = "elastic_beanstalk" + // EBServiceName is the key name for metadata of ElasticBeanstalkPlugin. + EBServiceName = "elastic_beanstalk" + + // EC2ServiceName is the key name for metadata of EC2Plugin. EC2ServiceName = "ec2" + + // ECSServiceName is the key name for metadata of ECSPlugin. ECSServiceName = "ecs" ) diff --git a/pattern/search_pattern.go b/pattern/search_pattern.go index 296e0d70..3422dd88 100644 --- a/pattern/search_pattern.go +++ b/pattern/search_pattern.go @@ -14,16 +14,16 @@ package pattern import "strings" // WildcardMatchCaseInsensitive returns true if text matches pattern (case-insensitive); returns false otherwise. -func WildcardMatchCaseInsensitive(pattern string, text string) bool { +func WildcardMatchCaseInsensitive(pattern, text string) bool { return WildcardMatch(pattern, text, true) } // WildcardMatch returns true if text matches pattern at the given case-sensitivity; returns false otherwise. -func WildcardMatch(pattern string, text string, caseInsensitive bool) bool { +func WildcardMatch(pattern, text string, caseInsensitive bool) bool { patternLen := len(pattern) textLen := len(text) - if 0 == patternLen { - return 0 == textLen + if patternLen == 0 { + return textLen == 0 } if pattern == "*" { @@ -41,23 +41,29 @@ func WildcardMatch(pattern string, text string, caseInsensitive bool) bool { pStar := 0 for i < textLen { - if p < patternLen && pattern[p] == text[i] { - i++ - p++ - } else if p < patternLen && '?' == pattern[p] { - i++ - p++ - } else if p < patternLen && pattern[p] == '*' { - iStar = i - pStar = p - p++ - } else if iStar != textLen { - iStar++ - i = iStar - p = pStar + 1 - } else { + if p < patternLen { + switch pattern[p] { + case text[i]: + i++ + p++ + continue + case '?': + i++ + p++ + continue + case '*': + iStar = i + pStar = p + p++ + continue + } + } + if iStar == textLen { return false } + iStar++ + i = iStar + p = pStar + 1 } for p < patternLen && pattern[p] == '*' { diff --git a/pattern/search_pattern_test.go b/pattern/search_pattern_test.go index c72a0a71..56f8aa70 100644 --- a/pattern/search_pattern_test.go +++ b/pattern/search_pattern_test.go @@ -28,7 +28,7 @@ func TestMatchExactNegative(t *testing.T) { assert.False(t, WildcardMatchCaseInsensitive("foo", "bar")) } -func TestSignleWildcardPositive(t *testing.T) { +func TestSingleWildcardPositive(t *testing.T) { assert.True(t, WildcardMatchCaseInsensitive("fo?", "foo")) } diff --git a/plugins/beanstalk/beanstalk.go b/plugins/beanstalk/beanstalk.go index c88183b1..c6814873 100644 --- a/plugins/beanstalk/beanstalk.go +++ b/plugins/beanstalk/beanstalk.go @@ -10,7 +10,8 @@ package beanstalk import "github.com/aws/aws-xray-sdk-go/awsplugins/beanstalk" -const Origin = "AWS::ElasticBeanstalk::Environment" +// Origin is the type of AWS resource that runs your application. +const Origin = beanstalk.Origin func init() { beanstalk.Init() diff --git a/plugins/ec2/ec2.go b/plugins/ec2/ec2.go index cdac2362..cd5f24c7 100644 --- a/plugins/ec2/ec2.go +++ b/plugins/ec2/ec2.go @@ -10,7 +10,8 @@ package ec2 import "github.com/aws/aws-xray-sdk-go/awsplugins/ec2" -const Origin = "AWS::EC2::Instance" +// Origin is the type of AWS resource that runs your application. +const Origin = ec2.Origin func init() { ec2.Init() diff --git a/plugins/ecs/ecs.go b/plugins/ecs/ecs.go index 51ed37ea..8d8d262a 100644 --- a/plugins/ecs/ecs.go +++ b/plugins/ecs/ecs.go @@ -10,7 +10,8 @@ package ecs import "github.com/aws/aws-xray-sdk-go/awsplugins/ecs" -const Origin = "AWS::ECS::Container" +// Origin is the type of AWS resource that runs your application. +const Origin = ecs.Origin func init() { ecs.Init() diff --git a/strategy/sampling/centralized.go b/strategy/sampling/centralized.go index 09ae593b..eb3ba08f 100644 --- a/strategy/sampling/centralized.go +++ b/strategy/sampling/centralized.go @@ -140,7 +140,7 @@ func (ss *CentralizedStrategy) ShouldTrace(request *Request) *Decision { logger.Debugf( "Determining ShouldTrace decision for:\n\thost: %s\n\tpath: %s\n\tmethod: %s\n\tservicename: %s\n\tservicetype: %s", request.Host, - request.Url, + request.URL, request.Method, request.ServiceName, request.ServiceType, diff --git a/strategy/sampling/centralized_test.go b/strategy/sampling/centralized_test.go index 85c02d87..54c0ef27 100644 --- a/strategy/sampling/centralized_test.go +++ b/strategy/sampling/centralized_test.go @@ -84,7 +84,7 @@ func TestShouldTracePositive1(t *testing.T) { sr := &Request{ Host: host1, - Url: url1, + URL: url1, Method: method1, ServiceName: serviceName1, ServiceType: servType1, @@ -180,7 +180,7 @@ func TestShouldTracePositive2(t *testing.T) { // serviceType missing sr := &Request{ Host: host1, - Url: url1, + URL: url1, Method: method1, ServiceName: serviceName1, } @@ -321,7 +321,7 @@ func TestShouldTraceDefaultPositive(t *testing.T) { sr := &Request{ Host: "www.foo.bar.com", - Url: "/resource/bat", + URL: "/resource/bat", Method: "GET", } @@ -415,7 +415,7 @@ func TestShouldTraceExpiredManifest(t *testing.T) { sr := &Request{ Host: "www.foo.bar.com", - Url: "/resource/bar", + URL: "/resource/bar", Method: "POST", } @@ -2597,7 +2597,7 @@ func TestLoadDaemonEndpoints1(t *testing.T) { sr := &Request{ Host: host1, - Url: url1, + URL: url1, Method: method1, ServiceName: serviceName1, ServiceType: servType1, @@ -2622,7 +2622,7 @@ func TestLoadDaemonEndpoints2(t *testing.T) { sr := &Request{ Host: host1, - Url: url1, + URL: url1, Method: method1, ServiceName: serviceName1, ServiceType: servType1, diff --git a/strategy/sampling/localized.go b/strategy/sampling/localized.go index df05cb66..c64b778c 100644 --- a/strategy/sampling/localized.go +++ b/strategy/sampling/localized.go @@ -60,10 +60,10 @@ func NewLocalizedStrategyFromJSONBytes(b []byte) (*LocalizedStrategy, error) { // ShouldTrace consults the LocalizedStrategy's rule set to determine // if the given request should be traced or not. func (lss *LocalizedStrategy) ShouldTrace(rq *Request) *Decision { - logger.Debugf("Determining ShouldTrace decision for:\n\thost: %s\n\tpath: %s\n\tmethod: %s", rq.Host, rq.Url, rq.Method) + logger.Debugf("Determining ShouldTrace decision for:\n\thost: %s\n\tpath: %s\n\tmethod: %s", rq.Host, rq.URL, rq.Method) if nil != lss.manifest.Rules { for _, r := range lss.manifest.Rules { - if r.AppliesTo(rq.Host, rq.Url, rq.Method) { + if r.AppliesTo(rq.Host, rq.URL, rq.Method) { logger.Debugf("Applicable rule:\n\tfixed_target: %d\n\trate: %f\n\thost: %s\n\turl_path: %s\n\thttp_method: %s", r.FixedTarget, r.Rate, r.Host, r.URLPath, r.HTTPMethod) return r.Sample() } diff --git a/strategy/sampling/reservoir.go b/strategy/sampling/reservoir.go index 2a017c61..81fba1ac 100644 --- a/strategy/sampling/reservoir.go +++ b/strategy/sampling/reservoir.go @@ -6,11 +6,12 @@ // // or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. -// Reservoirs allow a specified (`perSecond`) amount of `Take()`s per second. package sampling import "github.com/aws/aws-xray-sdk-go/utils" +// Reservoirs allow a specified (`perSecond`) amount of `Take()`s per second. + // reservoir is a set of properties common to all reservoirs type reservoir struct { // Total size of reservoir diff --git a/strategy/sampling/sampling_attributes.go b/strategy/sampling/sampling_attributes.go index cd1dc1c4..37cb8947 100644 --- a/strategy/sampling/sampling_attributes.go +++ b/strategy/sampling/sampling_attributes.go @@ -5,6 +5,7 @@ // http://aws.amazon.com/apache2.0/ // // or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + package sampling // Decision contains sampling decision and the rule matched for an incoming request @@ -17,7 +18,7 @@ type Decision struct { type Request struct { Host string Method string - Url string + URL string ServiceName string ServiceType string } diff --git a/strategy/sampling/sampling_rule.go b/strategy/sampling/sampling_rule.go index ae1b93a3..3faa16ab 100644 --- a/strategy/sampling/sampling_rule.go +++ b/strategy/sampling/sampling_rule.go @@ -40,7 +40,7 @@ func (p *Properties) AppliesTo(host, path, method string) bool { // Assumes lock is already held, if required. func (r *CentralizedRule) AppliesTo(request *Request) bool { return (request.Host == "" || pattern.WildcardMatchCaseInsensitive(r.Host, request.Host)) && - (request.Url == "" || pattern.WildcardMatchCaseInsensitive(r.URLPath, request.Url)) && + (request.URL == "" || pattern.WildcardMatchCaseInsensitive(r.URLPath, request.URL)) && (request.Method == "" || pattern.WildcardMatchCaseInsensitive(r.HTTPMethod, request.Method)) && (request.ServiceName == "" || pattern.WildcardMatchCaseInsensitive(r.ServiceName, request.ServiceName)) && (request.ServiceType == "" || pattern.WildcardMatchCaseInsensitive(r.serviceType, request.ServiceType)) @@ -190,7 +190,7 @@ func (r *CentralizedRule) snapshot() *xraySvc.SamplingStatisticsDocument { return s } -// Local Sampling Rule +// Rule is local sampling rule. type Rule struct { reservoir *Reservoir @@ -203,6 +203,7 @@ type Rule struct { mu sync.RWMutex } +// Sample is used to provide sampling decision. func (r *Rule) Sample() *Decision { var sd Decision r.mu.Lock() diff --git a/strategy/sampling/sampling_rule_manifest.go b/strategy/sampling/sampling_rule_manifest.go index a085ff69..cb2a1c4d 100644 --- a/strategy/sampling/sampling_rule_manifest.go +++ b/strategy/sampling/sampling_rule_manifest.go @@ -85,20 +85,20 @@ func initSamplingRules(srm *RuleManifest) { // processManifest returns the provided manifest if valid, or an error if the provided manifest is invalid. func processManifest(srm *RuleManifest) error { - if nil == srm { - return errors.New("Sampling rule manifest must not be nil.") + if srm == nil { + return errors.New("sampling rule manifest must not be nil") } - if 1 != srm.Version && 2 != srm.Version { - return errors.New(fmt.Sprintf("Sampling rule manifest version %d not supported.", srm.Version)) + if srm.Version != 1 && srm.Version != 2 { + return fmt.Errorf("sampling rule manifest version %d not supported", srm.Version) } - if nil == srm.Default { - return errors.New("Sampling rule manifest must include a default rule.") + if srm.Default == nil { + return errors.New("sampling rule manifest must include a default rule") } - if "" != srm.Default.URLPath || "" != srm.Default.ServiceName || "" != srm.Default.HTTPMethod { - return errors.New("The default rule must not specify values for url_path, service_name, or http_method.") + if srm.Default.URLPath != "" || srm.Default.ServiceName != "" || srm.Default.HTTPMethod != "" { + return errors.New("the default rule must not specify values for url_path, service_name, or http_method") } if srm.Default.FixedTarget < 0 || srm.Default.Rate < 0 { - return errors.New("The default rule must specify non-negative values for fixed_target and rate.") + return errors.New("the default rule must specify non-negative values for fixed_target and rate") } c := &utils.DefaultClock{} @@ -114,8 +114,7 @@ func processManifest(srm *RuleManifest) error { for _, r := range srm.Rules { if srm.Version == 1 { - err := validateVersion1(r) - if nil != err { + if err := validateVersion1(r); err != nil { return err } r.Host = r.ServiceName // V1 sampling rule contains service name and not host @@ -123,8 +122,7 @@ func processManifest(srm *RuleManifest) error { } if srm.Version == 2 { - err := validateVersion2(r) - if nil != err { + if err := validateVersion2(r); err != nil { return err } } diff --git a/strategy/sampling/sampling_test.go b/strategy/sampling/sampling_test.go index 9f957af6..27d11524 100644 --- a/strategy/sampling/sampling_test.go +++ b/strategy/sampling/sampling_test.go @@ -9,7 +9,7 @@ package sampling import ( - "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -22,33 +22,10 @@ func TestNewLocalizedStrategy(t *testing.T) { } func TestNewLocalizedStrategyFromFilePath1(t *testing.T) { // V1 sampling - ruleString := - `{ - "version": 1, - "default": { - "fixed_target": 1, - "rate": 0.05 - }, - "rules": [ - { - "description": "Example path-based rule below. Rules are evaluated in id-order, the default rule will be used if none match the incoming request. This is a rule for the checkout page.", - "id": "1", - "service_name": "*", - "http_method": "*", - "url_path": "/checkout", - "fixed_target": 10, - "rate": 0.05 - } - ] - }` - goPath := os.Getenv("PWD") - testFile := goPath + "/test_rule.json" - f, err := os.Create(testFile) + testFile, err := filepath.Abs(filepath.Join("testdata", "rule-v1-sampling.json")) if err != nil { - panic(err) + t.Fatal(err) } - f.WriteString(ruleString) - f.Close() ss, err := NewLocalizedStrategyFromFilePath(testFile) assert.NotNil(t, ss) assert.Equal(t, 1, ss.manifest.Version) @@ -61,37 +38,13 @@ func TestNewLocalizedStrategyFromFilePath1(t *testing.T) { // V1 sampling assert.Equal(t, 0.05, ss.manifest.Rules[0].Rate) assert.Nil(t, err) - os.Remove(testFile) } func TestNewLocalizedStrategyFromFilePath2(t *testing.T) { // V2 sampling - ruleString := - `{ - "version": 2, - "default": { - "fixed_target": 1, - "rate": 0.05 - }, - "rules": [ - { - "description": "Example path-based rule below. Rules are evaluated in id-order, the default rule will be used if none match the incoming request. This is a rule for the checkout page.", - "id": "1", - "host": "*", - "http_method": "*", - "url_path": "/checkout", - "fixed_target": 10, - "rate": 0.05 - } - ] - }` - goPath := os.Getenv("PWD") - testFile := goPath + "/test_rule.json" - f, err := os.Create(testFile) + testFile, err := filepath.Abs(filepath.Join("testdata", "rule-v2-sampling.json")) if err != nil { - panic(err) + t.Fatal(err) } - f.WriteString(ruleString) - f.Close() ss, err := NewLocalizedStrategyFromFilePath(testFile) assert.NotNil(t, ss) assert.Equal(t, 2, ss.manifest.Version) @@ -104,100 +57,36 @@ func TestNewLocalizedStrategyFromFilePath2(t *testing.T) { // V2 sampling assert.Equal(t, 0.05, ss.manifest.Rules[0].Rate) assert.Nil(t, err) - os.Remove(testFile) } func TestNewLocalizedStrategyFromFilePathInvalidRulesV1(t *testing.T) { // V1 contains host - ruleString := - `{ - "version": 1, - "default": { - "fixed_target": 1, - "rate": 0.05 - }, - "rules": [ - { - "description": "Example path-based rule below. Rules are evaluated in id-order, the default rule will be used if none match the incoming request. This is a rule for the checkout page.", - "id": "1", - "host": "*", - "http_method": "*", - "url_path": "/checkout", - "fixed_target": 10, - "rate": 0.05 - } - ] - }` - goPath := os.Getenv("PWD") - testFile := goPath + "/test_rule.json" - f, err := os.Create(testFile) + testFile, err := filepath.Abs(filepath.Join("testdata", "rule-v1-contains-host.json")) if err != nil { - panic(err) + t.Fatal(err) } - f.WriteString(ruleString) - f.Close() ss, err := NewLocalizedStrategyFromFilePath(testFile) assert.Nil(t, ss) assert.NotNil(t, err) - os.Remove(testFile) } func TestNewLocalizedStrategyFromFilePathInvalidRulesV2(t *testing.T) { // V2 contains service_name - ruleString := - `{ - "version": 2, - "default": { - "fixed_target": 1, - "rate": 0.05 - }, - "rules": [ - { - "description": "Example path-based rule below. Rules are evaluated in id-order, the default rule will be used if none match the incoming request. This is a rule for the checkout page.", - "id": "1", - "service_name": "*", - "http_method": "*", - "url_path": "/checkout", - "fixed_target": 10, - "rate": 0.05 - } - ] - }` - goPath := os.Getenv("PWD") - testFile := goPath + "/test_rule.json" - f, err := os.Create(testFile) + testFile, err := filepath.Abs(filepath.Join("testdata", "rule-v2-contains-service-name.json")) if err != nil { - panic(err) + t.Fatal(err) } - f.WriteString(ruleString) - f.Close() ss, err := NewLocalizedStrategyFromFilePath(testFile) assert.Nil(t, ss) assert.NotNil(t, err) - os.Remove(testFile) } func TestNewLocalizedStrategyFromFilePathWithInvalidJSON(t *testing.T) { // Test V1 sampling rule - ruleString := - `{ - "version": 1, - "default": { - "fixed_target": 1, - "rate": - }, - "rules": [ - ] - }` - goPath := os.Getenv("PWD") - testFile := goPath + "/test_rule.json" - f, err := os.Create(testFile) + testFile, err := filepath.Abs(filepath.Join("testdata", "rule-v1-invalid.json")) if err != nil { - panic(err) + t.Fatal(err) } - f.WriteString(ruleString) - f.Close() ss, err := NewLocalizedStrategyFromFilePath(testFile) assert.Nil(t, ss) assert.NotNil(t, err) - os.Remove(testFile) } func TestNewLocalizedStrategyFromJSONBytes(t *testing.T) { diff --git a/strategy/sampling/testdata/rule-v1-contains-host.json b/strategy/sampling/testdata/rule-v1-contains-host.json new file mode 100644 index 00000000..3e33eb90 --- /dev/null +++ b/strategy/sampling/testdata/rule-v1-contains-host.json @@ -0,0 +1,18 @@ +{ + "version": 1, + "default": { + "fixed_target": 1, + "rate": 0.05 + }, + "rules": [ + { + "description": "Example path-based rule below. Rules are evaluated in id-order, the default rule will be used if none match the incoming request. This is a rule for the checkout page.", + "id": "1", + "host": "*", + "http_method": "*", + "url_path": "/checkout", + "fixed_target": 10, + "rate": 0.05 + } + ] +} diff --git a/strategy/sampling/testdata/rule-v1-invalid.json b/strategy/sampling/testdata/rule-v1-invalid.json new file mode 100644 index 00000000..dd6b77c7 --- /dev/null +++ b/strategy/sampling/testdata/rule-v1-invalid.json @@ -0,0 +1,8 @@ +{ + "version": 1, + "default": { + "fixed_target": 1, + "rate": + }, + "rules": [] +} diff --git a/strategy/sampling/testdata/rule-v1-sampling.json b/strategy/sampling/testdata/rule-v1-sampling.json new file mode 100644 index 00000000..20e0f936 --- /dev/null +++ b/strategy/sampling/testdata/rule-v1-sampling.json @@ -0,0 +1,18 @@ +{ + "version": 1, + "default": { + "fixed_target": 1, + "rate": 0.05 + }, + "rules": [ + { + "description": "Example path-based rule below. Rules are evaluated in id-order, the default rule will be used if none match the incoming request. This is a rule for the checkout page.", + "id": "1", + "service_name": "*", + "http_method": "*", + "url_path": "/checkout", + "fixed_target": 10, + "rate": 0.05 + } + ] +} diff --git a/strategy/sampling/testdata/rule-v2-contains-service-name.json b/strategy/sampling/testdata/rule-v2-contains-service-name.json new file mode 100644 index 00000000..e86b9102 --- /dev/null +++ b/strategy/sampling/testdata/rule-v2-contains-service-name.json @@ -0,0 +1,18 @@ +{ + "version": 2, + "default": { + "fixed_target": 1, + "rate": 0.05 + }, + "rules": [ + { + "description": "Example path-based rule below. Rules are evaluated in id-order, the default rule will be used if none match the incoming request. This is a rule for the checkout page.", + "id": "1", + "service_name": "*", + "http_method": "*", + "url_path": "/checkout", + "fixed_target": 10, + "rate": 0.05 + } + ] +} diff --git a/strategy/sampling/testdata/rule-v2-sampling.json b/strategy/sampling/testdata/rule-v2-sampling.json new file mode 100644 index 00000000..d2d05456 --- /dev/null +++ b/strategy/sampling/testdata/rule-v2-sampling.json @@ -0,0 +1,18 @@ +{ + "version": 2, + "default": { + "fixed_target": 1, + "rate": 0.05 + }, + "rules": [ + { + "description": "Example path-based rule below. Rules are evaluated in id-order, the default rule will be used if none match the incoming request. This is a rule for the checkout page.", + "id": "1", + "host": "*", + "http_method": "*", + "url_path": "/checkout", + "fixed_target": 10, + "rate": 0.05 + } + ] +} diff --git a/utils/timer.go b/utils/timer.go index 3009f685..6cd1dfac 100644 --- a/utils/timer.go +++ b/utils/timer.go @@ -17,16 +17,19 @@ func init() { rand.Seed(time.Now().UnixNano()) } -type timer struct { +// Timer is the same as time.Timer except that it has jitters. +// A Timer must be created with NewTimer. +type Timer struct { t *time.Timer d time.Duration jitter time.Duration } -func NewTimer(d, jitter time.Duration) *timer { +// NewTimer creates a new Timer that will send the current time on its channel. +func NewTimer(d, jitter time.Duration) *Timer { t := time.NewTimer(d - time.Duration(rand.Int63n(int64(jitter)))) - jitteredTimer := timer{ + jitteredTimer := Timer{ t: t, d: d, jitter: jitter, @@ -35,10 +38,13 @@ func NewTimer(d, jitter time.Duration) *timer { return &jitteredTimer } -func (j *timer) C() <-chan time.Time { +// C is channel. +func (j *Timer) C() <-chan time.Time { return j.t.C } -func (j *timer) Reset() { +// Reset resets the timer. +// Reset should be invoked only on stopped or expired timers with drained channels. +func (j *Timer) Reset() { j.t.Reset(j.d - time.Duration(rand.Int63n(int64(j.jitter)))) } diff --git a/xray/aws.go b/xray/aws.go index d74289bf..a10d1c83 100644 --- a/xray/aws.go +++ b/xray/aws.go @@ -26,9 +26,16 @@ import ( "github.com/aws/aws-xray-sdk-go/resources" ) +// RequestIDKey is the key name of the request id. const RequestIDKey string = "request_id" + +// ExtendedRequestIDKey is the key name of the extend request id. const ExtendedRequestIDKey string = "id_2" + +// S3ExtendedRequestIDHeaderKey is the key name of the s3 extend request id. const S3ExtendedRequestIDHeaderKey string = "x-amz-id-2" + +// TraceIDHeaderKey is the HTTP header name used for tracing. const TraceIDHeaderKey = "x-amzn-trace-id" type jsonMap struct { diff --git a/xray/config.go b/xray/config.go index 8137e77a..cc5c8285 100644 --- a/xray/config.go +++ b/xray/config.go @@ -36,8 +36,8 @@ type SDK struct { RuleName string `json:"sampling_rule_name,omitempty"` } -// The logger instance used by xray. Only set from init() functions as -// SetLogger is not goroutine safe. +// SetLogger sets the logger instance used by xray. +// Only set from init() functions as SetLogger is not goroutine safe. func SetLogger(l xraylog.Logger) { logger.Logger = l } diff --git a/xray/default_streaming_strategy.go b/xray/default_streaming_strategy.go index 4962852f..b0eab9e3 100644 --- a/xray/default_streaming_strategy.go +++ b/xray/default_streaming_strategy.go @@ -71,9 +71,9 @@ func (dSS *DefaultStreamingStrategy) StreamCompletedSubsegments(seg *Segment) [] // Add extra information into child subsegment child.Lock() child.beforeEmitSubsegment(seg) - cb, err:= json.Marshal(child) - if err!= nil{ - logger.Errorf("JSON error while marshalling subsegment: %v",err) + cb, err := json.Marshal(child) + if err != nil { + logger.Errorf("JSON error while marshalling subsegment: %v", err) } outSegments = append(outSegments, cb) logger.Debugf("Streaming subsegment named '%s' from segment tree.", child.Name) diff --git a/xray/segment.go b/xray/segment.go index b3f6ffe1..f6a23787 100644 --- a/xray/segment.go +++ b/xray/segment.go @@ -12,7 +12,6 @@ import ( "context" "crypto/rand" "fmt" - "github.com/aws/aws-xray-sdk-go/strategy/sampling" "net/http" "os" "runtime" @@ -22,6 +21,7 @@ import ( "github.com/aws/aws-xray-sdk-go/header" "github.com/aws/aws-xray-sdk-go/internal/logger" "github.com/aws/aws-xray-sdk-go/internal/plugins" + "github.com/aws/aws-xray-sdk-go/strategy/sampling" ) // NewTraceID generates a string format of random trace ID. @@ -95,7 +95,7 @@ func BeginSegmentWithSampling(ctx context.Context, name string, r *http.Request, if traceHeader.SamplingDecision != header.Sampled && traceHeader.SamplingDecision != header.NotSampled { samplingRequest := &sampling.Request{ Host: r.Host, - Url: r.URL.Path, + URL: r.URL.Path, Method: r.Method, ServiceName: seg.Name, ServiceType: plugins.InstancePluginMetadata.Origin, @@ -109,10 +109,8 @@ func BeginSegmentWithSampling(ctx context.Context, name string, r *http.Request, if ctx.Done() != nil { go func() { - select { - case <-ctx.Done(): - seg.handleContextDone() - } + <-ctx.Done() + seg.handleContextDone() }() } @@ -281,26 +279,26 @@ func (seg *Segment) Close(err error) { } // CloseAndStream closes a subsegment and sends it. -func (subseg *Segment) CloseAndStream(err error) { - subseg.Lock() - defer subseg.Unlock() - - if subseg.parent != nil { - logger.Debugf("Ending subsegment named: %s", subseg.Name) - subseg.EndTime = float64(time.Now().UnixNano()) / float64(time.Second) - subseg.InProgress = false - subseg.Emitted = true - if subseg.parent.RemoveSubsegment(subseg) { - logger.Debugf("Removing subsegment named: %s", subseg.Name) +func (seg *Segment) CloseAndStream(err error) { + seg.Lock() + defer seg.Unlock() + + if seg.parent != nil { + logger.Debugf("Ending subsegment named: %s", seg.Name) + seg.EndTime = float64(time.Now().UnixNano()) / float64(time.Second) + seg.InProgress = false + seg.Emitted = true + if seg.parent.RemoveSubsegment(seg) { + logger.Debugf("Removing subsegment named: %s", seg.Name) } } if err != nil { - subseg.addError(err) + seg.addError(err) } - subseg.beforeEmitSubsegment(subseg.parent) - subseg.emit() + seg.beforeEmitSubsegment(seg.parent) + seg.emit() } // RemoveSubsegment removes a subsegment child from a segment or subsegment. @@ -441,12 +439,12 @@ func (seg *Segment) addSDKAndServiceInformation() { seg.GetService().CompilerVersion = runtime.Version() } -func (sub *Segment) beforeEmitSubsegment(seg *Segment) { +func (seg *Segment) beforeEmitSubsegment(s *Segment) { // Only called within a subsegment locked code block - sub.TraceID = seg.root().TraceID - sub.ParentID = seg.ID - sub.Type = "subsegment" - sub.RequestWasTraced = seg.RequestWasTraced + seg.TraceID = s.root().TraceID + seg.ParentID = s.ID + seg.Type = "subsegment" + seg.RequestWasTraced = s.RequestWasTraced } // AddAnnotation allows adding an annotation to the segment. diff --git a/xray/segment_test.go b/xray/segment_test.go index b0acf46a..1b683e6e 100644 --- a/xray/segment_test.go +++ b/xray/segment_test.go @@ -97,7 +97,7 @@ func TestSegmentDownstreamHeader(t *testing.T) { ctx, td := NewTestDaemon() defer td.Close() - ctx, seg := NewSegmentFromHeader(ctx, "TestSegment", &http.Request{URL:&url.URL{}}, &header.Header{ + ctx, seg := NewSegmentFromHeader(ctx, "TestSegment", &http.Request{URL: &url.URL{}}, &header.Header{ TraceID: "fakeid", ParentID: "reqid", }) diff --git a/xraylog/xray_log.go b/xraylog/xray_log.go index 4d9c439d..48ccdccc 100644 --- a/xraylog/xray_log.go +++ b/xraylog/xray_log.go @@ -21,7 +21,7 @@ import ( // to show up). type Logger interface { // Log can be called concurrently from multiple goroutines so make sure - // your implemenation is goroutine safe. + // your implementation is goroutine safe. Log(level LogLevel, msg fmt.Stringer) } @@ -31,9 +31,16 @@ type Logger interface { type LogLevel int const ( + // LogLevelDebug is usually only enabled when debugging. LogLevelDebug LogLevel = iota + 1 + + // LogLevelInfo is general operational entries about what's going on inside the application. LogLevelInfo + + // LogLevelWarn is non-critical entries that deserve eyes. LogLevelWarn + + // LogLevelError is used for errors that should definitely be noted. LogLevelError )