[receiver/apache] send port as a resource attribute (open-telemetry#1…

…6053)

* feat: add sending port as a resource attribute

.chloggen/apache-port-resource-attr.yaml

@@ -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: 

receiver/apachereceiver/README.md

@@ -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.

receiver/apachereceiver/config.go

@@ -27,7 +27,6 @@ import (
 type Config struct {
 	scraperhelper.ScraperControllerSettings `mapstructure:",squash"`
 	confighttp.HTTPClientSettings           `mapstructure:",squash"`
-	serverName                              string
 	Metrics                                 metadata.MetricsSettings `mapstructure:"metrics"`
 }
 
@@ -40,7 +39,6 @@ var (
 )
 
 func (cfg *Config) Validate() error {
-
 	u, err := url.Parse(cfg.Endpoint)
 	if err != nil {
 		return fmt.Errorf("invalid endpoint: '%s': %w", cfg.Endpoint, err)
@@ -54,6 +52,5 @@ func (cfg *Config) Validate() error {
 		return fmt.Errorf("query must be 'auto': '%s'", cfg.Endpoint)
 	}
 
-	cfg.serverName = u.Hostname()
 	return nil
 }

receiver/apachereceiver/documentation.md

@@ -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
 

receiver/apachereceiver/factory.go

@@ -16,6 +16,7 @@ package apachereceiver // import "github.com/open-telemetry/opentelemetry-collec
 
 import (
 	"context"
+	"net/url"
 	"time"
 
 	"go.opentelemetry.io/collector/component"
@@ -30,6 +31,9 @@ import (
 const (
 	typeStr   = "apache"
 	stability = component.StabilityLevelBeta
+
+	httpDefaultPort  = "80"
+	httpsDefaultPort = "443"
 )
 
 // NewFactory creates a factory for apache receiver.
@@ -54,15 +58,36 @@ func createDefaultConfig() config.Receiver {
 	}
 }
 
+func parseResourseAttributes(endpoint string) (string, string, error) {
+	u, err := url.Parse(endpoint)
+	serverName := u.Hostname()
+	port := u.Port()
+
+	if port == "" {
+		if u.Scheme == "https" {
+			port = httpsDefaultPort
+		} else if u.Scheme == "http" {
+			port = httpDefaultPort
+		}
+		// else: unknown scheme, leave port as empty string
+	}
+
+	return serverName, port, err
+}
+
 func createMetricsReceiver(
 	_ context.Context,
 	params component.ReceiverCreateSettings,
 	rConf config.Receiver,
 	consumer consumer.Metrics,
 ) (component.MetricsReceiver, error) {
 	cfg := rConf.(*Config)
+	serverName, port, err := parseResourseAttributes(cfg.Endpoint)
+	if err != nil {
+		return nil, err
+	}
 
-	ns := newApacheScraper(params, cfg)
+	ns := newApacheScraper(params, cfg, serverName, port)
 	scraper, err := scraperhelper.NewScraper(typeStr, ns.scrape, scraperhelper.WithStart(ns.start))
 	if err != nil {
 		return nil, err

receiver/apachereceiver/factory_test.go

@@ -52,3 +52,52 @@ func TestCreateMetricsReceiver(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, metricsReceiver)
 }
+
+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
+			_, port, err := parseResourseAttributes(tc.endpoint)
+
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedPort, port)
+		})
+	}
+}

receiver/apachereceiver/metadata.yaml

@@ -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:

receiver/apachereceiver/scraper.go

@@ -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 (
@@ -46,31 +48,45 @@ 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 {
 	settings   component.TelemetrySettings
 	cfg        *Config
 	httpClient *http.Client
 	mb         *metadata.MetricsBuilder
+	serverName string
+	port       string
 
 	// Feature gates regarding resource attributes
 	emitMetricsWithServerNameAsResourceAttribute bool
+	emitMetricsWithPortAsResourceAttribute       bool
 }
 
 func newApacheScraper(
 	settings component.ReceiverCreateSettings,
 	cfg *Config,
+	serverName string,
+	port string,
 ) *apacheScraper {
 	a := &apacheScraper{
-		settings: settings.TelemetrySettings,
-		cfg:      cfg,
-		mb:       metadata.NewMetricsBuilder(cfg.Metrics, settings.BuildInfo),
+		settings:   settings.TelemetrySettings,
+		cfg:        cfg,
+		mb:         metadata.NewMetricsBuilder(cfg.Metrics, settings.BuildInfo),
+		serverName: serverName,
+		port:       port,
 		emitMetricsWithServerNameAsResourceAttribute: featuregate.GetRegistry().IsEnabled(EmitServerNameAsResourceAttribute),
+		emitMetricsWithPortAsResourceAttribute:       featuregate.GetRegistry().IsEnabled(EmitPortAsResourceAttribute),
 	}
 
 	if !a.emitMetricsWithServerNameAsResourceAttribute {
@@ -79,6 +95,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
 }
 
@@ -106,11 +128,15 @@ func (r *apacheScraper) scrape(context.Context) (pmetric.Metrics, error) {
 
 	if r.emitMetricsWithServerNameAsResourceAttribute {
 		err = r.scrapeWithoutServerNameAttr(stats)
-		emitWith = append(emitWith, metadata.WithApacheServerName(r.cfg.serverName))
+		emitWith = append(emitWith, metadata.WithApacheServerName(r.serverName))
 	} else {
 		err = r.scrapeWithServerNameAttr(stats)
 	}
 
+	if r.emitMetricsWithPortAsResourceAttribute {
+		emitWith = append(emitWith, metadata.WithApacheServerPort(r.port))
+	}
+
 	return r.mb.Emit(emitWith...), err
 }
 
@@ -120,58 +146,58 @@ func (r *apacheScraper) scrapeWithServerNameAttr(stats string) error {
 	for metricKey, metricValue := range parseStats(stats) {
 		switch metricKey {
 		case "ServerUptimeSeconds":
-			addPartialIfError(errs, r.mb.RecordApacheUptimeDataPointWithServerName(now, metricValue, r.cfg.serverName))
+			addPartialIfError(errs, r.mb.RecordApacheUptimeDataPointWithServerName(now, metricValue, r.serverName))
 		case "ConnsTotal":
-			addPartialIfError(errs, r.mb.RecordApacheCurrentConnectionsDataPointWithServerName(now, metricValue, r.cfg.serverName))
+			addPartialIfError(errs, r.mb.RecordApacheCurrentConnectionsDataPointWithServerName(now, metricValue, r.serverName))
 		case "BusyWorkers":
-			addPartialIfError(errs, r.mb.RecordApacheWorkersDataPointWithServerName(now, metricValue, r.cfg.serverName,
+			addPartialIfError(errs, r.mb.RecordApacheWorkersDataPointWithServerName(now, metricValue, r.serverName,
 				metadata.AttributeWorkersStateBusy))
 		case "IdleWorkers":
-			addPartialIfError(errs, r.mb.RecordApacheWorkersDataPointWithServerName(now, metricValue, r.cfg.serverName,
+			addPartialIfError(errs, r.mb.RecordApacheWorkersDataPointWithServerName(now, metricValue, r.serverName,
 				metadata.AttributeWorkersStateIdle))
 		case "Total Accesses":
-			addPartialIfError(errs, r.mb.RecordApacheRequestsDataPointWithServerName(now, metricValue, r.cfg.serverName))
+			addPartialIfError(errs, r.mb.RecordApacheRequestsDataPointWithServerName(now, metricValue, r.serverName))
 		case "Total kBytes":
 			i, err := strconv.ParseInt(metricValue, 10, 64)
 			if err != nil {
 				errs.AddPartial(1, err)
 			} else {
-				r.mb.RecordApacheTrafficDataPointWithServerName(now, kbytesToBytes(i), r.cfg.serverName)
+				r.mb.RecordApacheTrafficDataPointWithServerName(now, kbytesToBytes(i), r.serverName)
 			}
 		case "CPUChildrenSystem":
 			addPartialIfError(
 				errs,
-				r.mb.RecordApacheCPUTimeDataPointWithServerName(now, metricValue, r.cfg.serverName, metadata.AttributeCPULevelChildren, metadata.AttributeCPUModeSystem),
+				r.mb.RecordApacheCPUTimeDataPointWithServerName(now, metricValue, r.serverName, metadata.AttributeCPULevelChildren, metadata.AttributeCPUModeSystem),
 			)
 		case "CPUChildrenUser":
 			addPartialIfError(
 				errs,
-				r.mb.RecordApacheCPUTimeDataPointWithServerName(now, metricValue, r.cfg.serverName, metadata.AttributeCPULevelChildren, metadata.AttributeCPUModeUser),
+				r.mb.RecordApacheCPUTimeDataPointWithServerName(now, metricValue, r.serverName, metadata.AttributeCPULevelChildren, metadata.AttributeCPUModeUser),
 			)
 		case "CPUSystem":
 			addPartialIfError(
 				errs,
-				r.mb.RecordApacheCPUTimeDataPointWithServerName(now, metricValue, r.cfg.serverName, metadata.AttributeCPULevelSelf, metadata.AttributeCPUModeSystem),
+				r.mb.RecordApacheCPUTimeDataPointWithServerName(now, metricValue, r.serverName, metadata.AttributeCPULevelSelf, metadata.AttributeCPUModeSystem),
 			)
 		case "CPUUser":
 			addPartialIfError(
 				errs,
-				r.mb.RecordApacheCPUTimeDataPointWithServerName(now, metricValue, r.cfg.serverName, metadata.AttributeCPULevelSelf, metadata.AttributeCPUModeUser),
+				r.mb.RecordApacheCPUTimeDataPointWithServerName(now, metricValue, r.serverName, metadata.AttributeCPULevelSelf, metadata.AttributeCPUModeUser),
 			)
 		case "CPULoad":
-			addPartialIfError(errs, r.mb.RecordApacheCPULoadDataPointWithServerName(now, metricValue, r.cfg.serverName))
+			addPartialIfError(errs, r.mb.RecordApacheCPULoadDataPointWithServerName(now, metricValue, r.serverName))
 		case "Load1":
-			addPartialIfError(errs, r.mb.RecordApacheLoad1DataPointWithServerName(now, metricValue, r.cfg.serverName))
+			addPartialIfError(errs, r.mb.RecordApacheLoad1DataPointWithServerName(now, metricValue, r.serverName))
 		case "Load5":
-			addPartialIfError(errs, r.mb.RecordApacheLoad5DataPointWithServerName(now, metricValue, r.cfg.serverName))
+			addPartialIfError(errs, r.mb.RecordApacheLoad5DataPointWithServerName(now, metricValue, r.serverName))
 		case "Load15":
-			addPartialIfError(errs, r.mb.RecordApacheLoad15DataPointWithServerName(now, metricValue, r.cfg.serverName))
+			addPartialIfError(errs, r.mb.RecordApacheLoad15DataPointWithServerName(now, metricValue, r.serverName))
 		case "Total Duration":
-			addPartialIfError(errs, r.mb.RecordApacheRequestTimeDataPointWithServerName(now, metricValue, r.cfg.serverName))
+			addPartialIfError(errs, r.mb.RecordApacheRequestTimeDataPointWithServerName(now, metricValue, r.serverName))
 		case "Scoreboard":
 			scoreboardMap := parseScoreboard(metricValue)
 			for state, score := range scoreboardMap {
-				r.mb.RecordApacheScoreboardDataPointWithServerName(now, score, r.cfg.serverName, state)
+				r.mb.RecordApacheScoreboardDataPointWithServerName(now, score, r.serverName, state)
 			}
 		}
 	}