Skip to content

Commit

Permalink
feat: add sending port as a resource attribute
Browse files Browse the repository at this point in the history
  • Loading branch information
aboguszewski-sumo committed Nov 3, 2022
1 parent 6de7397 commit 748aadc
Show file tree
Hide file tree
Showing 10 changed files with 135 additions and 3 deletions.
16 changes: 16 additions & 0 deletions .chloggen/apache-port-resource-attr.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: apachereceiver

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: add port resource attribute

# One or more tracking issues related to the change
issues: [14791]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
10 changes: 10 additions & 0 deletions receiver/apachereceiver/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,13 @@ This is considered a breaking change for existing users of this receiver, and it
This feature gate will eventually be enabled by default, and eventually the old implementation will be removed. It aims
to give users time to migrate to the new implementation. The target release for this featuregate to be enabled by default
is 0.66.0.

**ALPHA**: `receiver.apache.emitPortAsResourceAttribute`

The feature gate `receiver.apache.emitPortAsResourceAttribute` once enabled starts emitting the metrics with a resource attribute `apache.server.port`.

This is considered a breaking change for existing users of this receiver, and it is recommended to migrate to the new implementation when possible. Any new users planning to adopt this receiver should enable this feature gate to avoid having to migrate any visualisations or alerts.

This feature gate will eventually be enabled by default, and eventually the old implementation will be removed. It aims
to give users time to migrate to the new implementation. The target release for this featuregate to be enabled by default
is 0.66.0.
15 changes: 15 additions & 0 deletions receiver/apachereceiver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type Config struct {
scraperhelper.ScraperControllerSettings `mapstructure:",squash"`
confighttp.HTTPClientSettings `mapstructure:",squash"`
serverName string
port string
Metrics metadata.MetricsSettings `mapstructure:"metrics"`
}

Expand All @@ -37,6 +38,9 @@ var (
defaultPort = "8080"
defaultPath = "server-status"
defaultEndpoint = fmt.Sprintf("%s%s:%s/%s?auto", defaultProtocol, defaultHost, defaultPort, defaultPath)

httpDefaultPort = "80"
httpsDefaultPort = "443"
)

func (cfg *Config) Validate() error {
Expand All @@ -55,5 +59,16 @@ func (cfg *Config) Validate() error {
}

cfg.serverName = u.Hostname()
cfg.port = u.Port()

if cfg.port == "" {
if u.Scheme == "https" {
cfg.port = httpsDefaultPort
} else if u.Scheme == "http" {
cfg.port = httpDefaultPort
}
// else: unknown scheme, leave port as empty string
}

return nil
}
49 changes: 49 additions & 0 deletions receiver/apachereceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,52 @@ func TestLoadConfig(t *testing.T) {

require.Equal(t, expected, cfg)
}

func TestPortValidate(t *testing.T) {
testCases := []struct {
desc string
endpoint string
expectedPort string
}{
{
desc: "http with specified port",
endpoint: "http://localhost:8080/server-status?auto",
expectedPort: "8080",
},
{
desc: "http without specified port",
endpoint: "http://localhost/server-status?auto",
expectedPort: "80",
},
{
desc: "https with specified port",
endpoint: "https://localhost:8080/server-status?auto",
expectedPort: "8080",
},
{
desc: "https without specified port",
endpoint: "https://localhost/server-status?auto",
expectedPort: "443",
},
{
desc: "unknown protocol with specified port",
endpoint: "abc://localhost:8080/server-status?auto",
expectedPort: "8080",
},
{
desc: "port unresolvable",
endpoint: "abc://localhost/server-status?auto",
expectedPort: "",
},
}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
cfg := NewFactory().CreateDefaultConfig().(*Config)
cfg.Endpoint = tc.endpoint
err := cfg.Validate()

require.NoError(t, err)
require.Equal(t, tc.expectedPort, cfg.port)
})
}
}
1 change: 1 addition & 0 deletions receiver/apachereceiver/documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ metrics:
| Name | Description | Type |
| ---- | ----------- | ---- |
| apache.server.name | The name of the Apache HTTP server. | Str |
| apache.server.port | The port of the Apache HTTP server. | Str |
## Metric attributes
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions receiver/apachereceiver/metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ resource_attributes:
apache.server.name:
description: The name of the Apache HTTP server.
type: string
apache.server.port:
description: The port of the Apache HTTP server.
type: string

attributes:
workers_state:
Expand Down
20 changes: 20 additions & 0 deletions receiver/apachereceiver/scraper.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ import (
const (
readmeURL = "https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/apachereceiver/README.md"
EmitServerNameAsResourceAttribute = "receiver.apache.emitServerNameAsResourceAttribute"
EmitPortAsResourceAttribute = "receiver.apache.emitPortAsResourceAttribute"
featureGateWarning = "Feature gate %s is not enabled. Please see the README.md file of apache receiver for more information."
)

var (
Expand All @@ -46,10 +48,16 @@ var (
Description: "When enabled, the name of the server will be sent as an apache.server.name resource attribute " +
"instead of a metric-level server_name attribute.",
}
emitPortAsResourceAttribute = featuregate.Gate{
ID: EmitPortAsResourceAttribute,
Enabled: false,
Description: "When enabled, the port of the server will be sent as an apache.server.name resource attribute.",
}
)

func init() {
featuregate.GetRegistry().MustRegister(emitServerNameAsResourceAttribute)
featuregate.GetRegistry().MustRegister(emitPortAsResourceAttribute)
}

type apacheScraper struct {
Expand All @@ -60,6 +68,7 @@ type apacheScraper struct {

// Feature gates regarding resource attributes
emitMetricsWithServerNameAsResourceAttribute bool
emitMetricsWithPortAsResourceAttribute bool
}

func newApacheScraper(
Expand All @@ -71,6 +80,7 @@ func newApacheScraper(
cfg: cfg,
mb: metadata.NewMetricsBuilder(cfg.Metrics, settings.BuildInfo),
emitMetricsWithServerNameAsResourceAttribute: featuregate.GetRegistry().IsEnabled(EmitServerNameAsResourceAttribute),
emitMetricsWithPortAsResourceAttribute: featuregate.GetRegistry().IsEnabled(EmitPortAsResourceAttribute),
}

if !a.emitMetricsWithServerNameAsResourceAttribute {
Expand All @@ -79,6 +89,12 @@ func newApacheScraper(
)
}

if !a.emitMetricsWithPortAsResourceAttribute {
settings.Logger.Warn(
fmt.Sprintf("Feature gate %s is not enabled. Please see the README for more information: %s", EmitPortAsResourceAttribute, readmeURL),
)
}

return a
}

Expand Down Expand Up @@ -111,6 +127,10 @@ func (r *apacheScraper) scrape(context.Context) (pmetric.Metrics, error) {
err = r.scrapeWithServerNameAttr(stats)
}

if r.emitMetricsWithPortAsResourceAttribute {
emitWith = append(emitWith, metadata.WithApacheServerPort(r.cfg.port))
}

return r.mb.Emit(emitWith...), err
}

Expand Down
9 changes: 7 additions & 2 deletions receiver/apachereceiver/scraper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"net/url"
"path/filepath"
"testing"

Expand All @@ -41,8 +42,7 @@ func TestScraper(t *testing.T) {
require.NoError(t, cfg.Validate())

// Let this test check if it works with the feature enabled and the integration test will test the feature disabled.
err := featuregate.GetRegistry().Apply(map[string]bool{EmitServerNameAsResourceAttribute: true})

err := featuregate.GetRegistry().Apply(map[string]bool{EmitServerNameAsResourceAttribute: true, EmitPortAsResourceAttribute: true})
require.NoError(t, err)

scraper := newApacheScraper(componenttest.NewNopReceiverCreateSettings(), cfg)
Expand All @@ -56,7 +56,12 @@ func TestScraper(t *testing.T) {
expectedFile := filepath.Join("testdata", "scraper", "expected.json")
expectedMetrics, err := golden.ReadMetrics(expectedFile)
require.NoError(t, err)
url, err := url.Parse(apacheMock.URL)
require.NoError(t, err)
expectedMetrics.ResourceMetrics().At(0).Resource().Attributes().Remove("apache.server.port")
expectedMetrics.ResourceMetrics().At(0).Resource().Attributes().PutStr("apache.server.port", url.Port())

// The port is random, so we shouldn't check if this value matches.
require.NoError(t, scrapertest.CompareMetrics(expectedMetrics, actualMetrics))
}

Expand Down
8 changes: 7 additions & 1 deletion receiver/apachereceiver/testdata/scraper/expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@
"value": {
"stringValue": "127.0.0.1"
}
}
},
{
"key": "apache.server.port",
"value": {
"stringValue": "8080"
}
}
]
},
"scopeMetrics": [
Expand Down

0 comments on commit 748aadc

Please sign in to comment.