Skip to content

Commit 19ade87

Browse files
berkitamasulikl
berkitamas
authored and
ulikl
committed
Fixes after review
Signed-off-by: Tamas Berki <[email protected]>
1 parent 7a1cd7f commit 19ade87

File tree

2 files changed

+98
-139
lines changed

2 files changed

+98
-139
lines changed

‎prober/http.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ func ProbeHTTP(ctx context.Context, target string, module config.Module, registr
390390
targetPort := targetURL.Port()
391391

392392
var ip *net.IPAddr
393-
if !module.HTTP.SkipResolvePhaseWithProxy || (module.HTTP.HTTPClientConfig.ProxyConfig.ProxyURL.URL == nil && !module.HTTP.HTTPClientConfig.ProxyConfig.ProxyFromEnvironment) {
393+
if shouldResolveDNSWithProxy(module.HTTP) {
394394
var lookupTime float64
395395
ip, lookupTime, err = chooseProtocol(ctx, module.HTTP.IPProtocol, module.HTTP.IPProtocolFallback, targetHost, registry, logger)
396396
durationGaugeVec.WithLabelValues("resolve").Add(lookupTime)
@@ -754,3 +754,9 @@ func getDecompressionReader(algorithm string, origBody io.ReadCloser) (io.ReadCl
754754
return nil, errors.New("unsupported compression algorithm")
755755
}
756756
}
757+
758+
// Returns true if DNS requests should go through a proxy
759+
func shouldResolveDNSWithProxy(httpProbe config.HTTPProbe) bool {
760+
proxySet := httpProbe.HTTPClientConfig.ProxyURL.URL != nil || httpProbe.HTTPClientConfig.ProxyFromEnvironment
761+
return !httpProbe.SkipResolvePhaseWithProxy || !proxySet
762+
}

‎prober/http_test.go

Lines changed: 91 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -1651,6 +1651,13 @@ func TestCookieJar(t *testing.T) {
16511651
}
16521652

16531653
func TestSkipResolvePhase(t *testing.T) {
1654+
type testdata struct {
1655+
skipResolve bool
1656+
proxyFromEnv bool
1657+
proxyFromConfig bool
1658+
expectResolve bool
1659+
}
1660+
16541661
if testing.Short() {
16551662
t.Skip("skipping network dependent test")
16561663
}
@@ -1659,151 +1666,97 @@ func TestSkipResolvePhase(t *testing.T) {
16591666
}))
16601667
defer ts.Close()
16611668

1662-
// Note: just tested, if local resolve is done or not via metrics. No proxy with diffent resolving available.
1669+
testcases := map[string]testdata{
1670+
"Without Proxy with skipping resolve": {
1671+
skipResolve: true,
1672+
proxyFromEnv: false,
1673+
proxyFromConfig: false,
1674+
expectResolve: true,
1675+
},
1676+
"Without Proxy without skipping resolve": {
1677+
skipResolve: false,
1678+
proxyFromEnv: false,
1679+
proxyFromConfig: false,
1680+
expectResolve: true,
1681+
},
1682+
"With Proxy config without skipping resolve": {
1683+
skipResolve: false,
1684+
proxyFromEnv: false,
1685+
proxyFromConfig: true,
1686+
expectResolve: true,
1687+
},
1688+
"With Proxy config with skipping resolve": {
1689+
skipResolve: true,
1690+
proxyFromEnv: false,
1691+
proxyFromConfig: true,
1692+
expectResolve: false,
1693+
},
1694+
"With Proxy env without skipping resolve": {
1695+
skipResolve: false,
1696+
proxyFromEnv: true,
1697+
proxyFromConfig: false,
1698+
expectResolve: true,
1699+
},
1700+
"With Proxy env with skipping resolve": {
1701+
skipResolve: true,
1702+
proxyFromEnv: true,
1703+
proxyFromConfig: false,
1704+
expectResolve: false,
1705+
},
1706+
}
16631707

1664-
t.Run("Without Proxy", func(t *testing.T) {
1665-
registry := prometheus.NewRegistry()
1666-
testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second)
1667-
defer cancel()
1668-
result := ProbeHTTP(testCTX, ts.URL,
1669-
config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, HTTPClientConfig: pconfig.DefaultHTTPClientConfig, SkipResolvePhaseWithProxy: true}}, registry, promslog.NewNopLogger())
1670-
if !result {
1671-
t.Fatalf("Probe unsuccessful")
1672-
}
1673-
mfs, err := registry.Gather()
1674-
if err != nil {
1675-
t.Fatal(err)
1676-
}
1677-
expectedMetrics := map[string]map[string]map[string]struct{}{
1678-
"probe_http_duration_seconds": {
1679-
"phase": {
1680-
"connect": {},
1681-
"processing": {},
1682-
"resolve": {},
1683-
"transfer": {},
1684-
"tls": {},
1685-
},
1686-
},
1687-
}
1708+
for name, tc := range testcases {
1709+
t.Run(name, func(t *testing.T) {
1710+
registry := prometheus.NewRegistry()
1711+
testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second)
1712+
defer cancel()
1713+
httpCfg := pconfig.DefaultHTTPClientConfig
1714+
if tc.proxyFromConfig {
1715+
u, err := url.Parse("http://127.0.0.1:3128")
1716+
if err != nil {
1717+
t.Fatal(err.Error())
1718+
}
1719+
httpCfg.ProxyURL = pconfig.URL{
1720+
URL: u,
1721+
}
1722+
}
1723+
if tc.proxyFromEnv {
1724+
t.Setenv("HTTP_PROXY", "http://127.0.0.1:3128")
1725+
httpCfg.ProxyFromEnvironment = true
1726+
}
16881727

1689-
checkMetrics(expectedMetrics, mfs, t)
1690-
})
1691-
t.Run("With Proxy and resolve", func(t *testing.T) {
1692-
registry := prometheus.NewRegistry()
1693-
testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second)
1694-
defer cancel()
1695-
httpCfg := pconfig.DefaultHTTPClientConfig
1696-
u, err := url.Parse("http://127.0.0.1:3128")
1697-
if err != nil {
1698-
t.Fatal(err.Error())
1699-
}
1700-
httpCfg.ProxyURL = pconfig.URL{
1701-
URL: u,
1702-
}
1703-
ProbeHTTP(testCTX, ts.URL,
1704-
config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, HTTPClientConfig: httpCfg, SkipResolvePhaseWithProxy: false}}, registry, promslog.NewNopLogger())
1705-
mfs, err := registry.Gather()
1706-
if err != nil {
1707-
t.Fatal(err)
1708-
}
1709-
expectedMetrics := map[string]map[string]map[string]struct{}{
1710-
"probe_http_duration_seconds": {
1711-
"phase": {
1712-
"connect": {},
1713-
"processing": {},
1714-
"resolve": {},
1715-
"transfer": {},
1716-
"tls": {},
1717-
},
1718-
},
1719-
}
1728+
result := ProbeHTTP(testCTX, ts.URL,
1729+
config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, HTTPClientConfig: httpCfg, SkipResolvePhaseWithProxy: tc.skipResolve}}, registry, promslog.NewNopLogger())
17201730

1721-
checkMetrics(expectedMetrics, mfs, t)
1722-
})
1723-
t.Run("With Proxy and without resolve", func(t *testing.T) {
1724-
registry := prometheus.NewRegistry()
1725-
testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second)
1726-
defer cancel()
1727-
httpCfg := pconfig.DefaultHTTPClientConfig
1728-
u, err := url.Parse("http://127.0.0.1:3128")
1729-
if err != nil {
1730-
t.Fatal(err.Error())
1731-
}
1732-
httpCfg.ProxyURL = pconfig.URL{
1733-
URL: u,
1734-
}
1735-
ProbeHTTP(testCTX, ts.URL,
1736-
config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, HTTPClientConfig: httpCfg, SkipResolvePhaseWithProxy: true}}, registry, promslog.NewNopLogger())
1737-
mfs, err := registry.Gather()
1738-
if err != nil {
1739-
t.Fatal(err)
1740-
}
1741-
expectedMetrics := map[string]map[string]map[string]struct{}{
1742-
"probe_http_duration_seconds": {
1743-
"phase": {
1744-
"connect": {},
1745-
"processing": {},
1746-
"transfer": {},
1747-
"tls": {},
1748-
},
1749-
},
1750-
}
1731+
// Skip probe output check in case of proxy, as there is no test proxy running on that endpoint
1732+
if !tc.proxyFromConfig && !tc.proxyFromEnv {
1733+
if !result {
1734+
t.Fatalf("Probe unsuccessful")
1735+
}
1736+
}
17511737

1752-
checkMetrics(expectedMetrics, mfs, t)
1753-
})
1754-
t.Run("With Proxy from env var and without resolve", func(t *testing.T) {
1755-
registry := prometheus.NewRegistry()
1756-
testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second)
1757-
defer cancel()
1758-
t.Setenv("HTTP_PROXY", "http://127.0.0.1:3128")
1759-
httpCfg := pconfig.DefaultHTTPClientConfig
1760-
httpCfg.ProxyFromEnvironment = true
1761-
ProbeHTTP(testCTX, ts.URL,
1762-
config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, HTTPClientConfig: httpCfg, SkipResolvePhaseWithProxy: true}}, registry, promslog.NewNopLogger())
1763-
mfs, err := registry.Gather()
1764-
if err != nil {
1765-
t.Fatal(err)
1766-
}
1767-
expectedMetrics := map[string]map[string]map[string]struct{}{
1768-
"probe_http_duration_seconds": {
1769-
"phase": {
1770-
"connect": {},
1771-
"processing": {},
1772-
"transfer": {},
1773-
"tls": {},
1774-
},
1775-
},
1776-
}
1738+
mfs, err := registry.Gather()
1739+
if err != nil {
1740+
t.Fatal(err)
1741+
}
17771742

1778-
checkMetrics(expectedMetrics, mfs, t)
1779-
})
1780-
t.Run("With Proxy from env var and with resolve", func(t *testing.T) {
1781-
registry := prometheus.NewRegistry()
1782-
testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second)
1783-
defer cancel()
1784-
t.Setenv("HTTP_PROXY", "http://127.0.0.1:3128")
1785-
httpCfg := pconfig.DefaultHTTPClientConfig
1786-
httpCfg.ProxyFromEnvironment = true
1787-
ProbeHTTP(testCTX, ts.URL,
1788-
config.Module{Timeout: time.Second, HTTP: config.HTTPProbe{IPProtocolFallback: true, HTTPClientConfig: httpCfg, SkipResolvePhaseWithProxy: false}}, registry, promslog.NewNopLogger())
1789-
mfs, err := registry.Gather()
1790-
if err != nil {
1791-
t.Fatal(err)
1792-
}
1793-
expectedMetrics := map[string]map[string]map[string]struct{}{
1794-
"probe_http_duration_seconds": {
1795-
"phase": {
1796-
"connect": {},
1797-
"processing": {},
1798-
"resolve": {},
1799-
"transfer": {},
1800-
"tls": {},
1743+
expectedMetrics := map[string]map[string]map[string]struct{}{
1744+
"probe_http_duration_seconds": {
1745+
"phase": {
1746+
"connect": {},
1747+
"processing": {},
1748+
"transfer": {},
1749+
"tls": {},
1750+
},
18011751
},
1802-
},
1803-
}
1752+
}
1753+
if tc.expectResolve {
1754+
expectedMetrics["probe_http_duration_seconds"]["phase"]["resolve"] = struct{}{}
1755+
}
18041756

1805-
checkMetrics(expectedMetrics, mfs, t)
1806-
})
1757+
checkMetrics(expectedMetrics, mfs, t)
1758+
})
1759+
}
18071760
}
18081761

18091762
func TestBody(t *testing.T) {

0 commit comments

Comments
 (0)