Skip to content

Commit

Permalink
cmd/ocagent: fix Zipkin logical conflict detection typo
Browse files Browse the repository at this point in the history
The Zipkin logical conflict detection had 2 errors:
a) By mistake, it resolved the receiver{Host, Port}
from the exporterHostPort address
b) During IP resolution to check say if "localhost" and "//zipkin"
were the same, if the results were the same, the ports weren't
checked for equivalence hence if we had:
    "localhost:9410" and "127.0.0.1:9411"
it would incorrectly report a logical conflict.

Added a regression test as well.

Fixes #233
  • Loading branch information
odeke-em committed Nov 28, 2018
1 parent 7e8cb2d commit 9b7718d
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 2 deletions.
4 changes: 2 additions & 2 deletions cmd/ocagent/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (c *config) checkLogicalConflicts(blob []byte) error {
zExporterHostPort, zReceiverHostPort)
}
zExpHost, zExpPort, _ := net.SplitHostPort(zExporterHostPort)
zReceiverHost, zReceiverPort, _ := net.SplitHostPort(zExporterHostPort)
zReceiverHost, zReceiverPort, _ := net.SplitHostPort(zReceiverHostPort)
if eqHosts(zExpHost, zReceiverHost) && zExpPort == zReceiverPort {
return fmt.Errorf("ZipkinExporter address (%q) aka (%s on port %s)\nis the same as the receiver address (%q) aka (%s on port %s)",
zExporterHostPort, zExpHost, zExpPort, zReceiverHostPort, zReceiverHost, zReceiverPort)
Expand All @@ -192,7 +192,7 @@ func (c *config) checkLogicalConflicts(blob []byte) error {
// Otherwise, now let's resolve the IPs and ensure that they aren't the same
zExpIPAddr, _ := net.ResolveIPAddr("ip", zExpHost)
zReceiverIPAddr, _ := net.ResolveIPAddr("ip", zReceiverHost)
if zExpIPAddr != nil && zReceiverIPAddr != nil && reflect.DeepEqual(zExpIPAddr, zReceiverIPAddr) {
if zExpIPAddr != nil && zReceiverIPAddr != nil && reflect.DeepEqual(zExpIPAddr, zReceiverIPAddr) && zExpPort == zReceiverPort {
return fmt.Errorf("ZipkinExporter address (%q) aka (%+v)\nis the same as the\nreceiver address (%q) aka (%+v)",
zExporterHostPort, zExpIPAddr, zReceiverHostPort, zReceiverIPAddr)
}
Expand Down
62 changes: 62 additions & 0 deletions cmd/ocagent/config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright 2018, OpenCensus Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License 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 main

import (
"testing"

yaml "gopkg.in/yaml.v2"

"github.com/census-instrumentation/opencensus-service/exporter/exporterparser"
)

// Issue #233: Zipkin receiver and exporter loopback detection
// would mistakenly report that "localhost:9410" and "localhost:9411"
// were equal, due to a mistake in parsing out their addresses,
// but also after IP resolution, the equivalence of ports was not being
// checked.
func TestZipkinReceiverExporterLogicalConflictChecks(t *testing.T) {
regressionYAML := []byte(`
receivers:
zipkin:
address: "localhost:9410"
exporters:
zipkin:
endpoint: "http://localhost:9411/api/v2/spans"
`)

cfg, err := parseOCAgentConfig(regressionYAML)
if err != nil {
t.Fatalf("Unexpected YAML parse error: %v", err)
}
if err := cfg.checkLogicalConflicts(regressionYAML); err != nil {
t.Fatalf("Unexpected error: %v", err)
}

if g, w := cfg.Receivers.Zipkin.Address, "localhost:9410"; g != w {
t.Errorf("Receivers.Zipkin.EndpointURL mismatch\nGot: %s\nWant:%s", g, w)
}

var ecfg struct {
Exporters *struct {
Zipkin *exporterparser.ZipkinConfig `yaml:"zipkin"`
} `yaml:"exporters"`
}
_ = yaml.Unmarshal(regressionYAML, &ecfg)
if g, w := ecfg.Exporters.Zipkin.EndpointURL(), "http://localhost:9411/api/v2/spans"; g != w {
t.Errorf("Exporters.Zipkin.EndpointURL mismatch\nGot: %s\nWant:%s", g, w)
}
}

0 comments on commit 9b7718d

Please sign in to comment.