Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[exporter/awsemf] Renaming APM/Pulse to AppSignals #126

Merged
merged 1 commit into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions exporter/awsemfexporter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,12 @@ func (config *Config) IsEnhancedContainerInsights() bool {
// return config.EnhancedContainerInsights && !config.DisableMetricExtraction
}

func (config *Config) IsPulseApmEnabled() bool {
func (config *Config) IsAppSignalsEnabled() bool {
if config.LogGroupName == "" || config.Namespace == "" {
return false
}

if config.Namespace == pulseMetricNamespace && strings.HasPrefix(config.LogGroupName, pulseLogGroupNamePrefix) {
if config.Namespace == appSignalsMetricNamespace && strings.HasPrefix(config.LogGroupName, appSignalsLogGroupNamePrefix) {
return true
}

Expand Down
28 changes: 14 additions & 14 deletions exporter/awsemfexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,35 +328,35 @@ func TestIsEnhancedContainerInsights(t *testing.T) {
assert.False(t, cfg.IsEnhancedContainerInsights())
}

func TestIsPulseApmEnabled(t *testing.T) {
func TestIsAppSignalsEnabled(t *testing.T) {
tests := []struct {
name string
metricNameSpace string
logGroupName string
expectedResult bool
}{
{
"validPulseEMF",
"AWS/APM",
"/aws/apm/eks",
"validAppSignalsEMF",
"AppSignals",
"/aws/appsignals/eks",
true,
},
{
"invalidPulseLogsGroup",
"AWS/APM",
"/nonaws/apm/eks",
"invalidAppSignalsLogsGroup",
"AppSignals",
"/nonaws/appsignals/eks",
false,
},
{
"invalidPulseMetricNamespace",
"NonAWS/APM",
"/aws/apm/eks",
"invalidAppSignalsMetricNamespace",
"NonAppSignals",
"/aws/appsignals/eks",
false,
},
{
"invalidPulseEMF",
"NonAWS/APM",
"/nonaws/apm/eks",
"invalidAppSignalsEMF",
"NonAppSignals",
"/nonaws/appsignals/eks",
false,
},
{
Expand All @@ -377,7 +377,7 @@ func TestIsPulseApmEnabled(t *testing.T) {
cfg.LogGroupName = tc.logGroupName
}

assert.Equal(t, cfg.IsPulseApmEnabled(), tc.expectedResult)
assert.Equal(t, cfg.IsAppSignalsEnabled(), tc.expectedResult)
})
}
}
8 changes: 4 additions & 4 deletions exporter/awsemfexporter/emf_exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ const (
outputDestinationCloudWatch = "cloudwatch"
outputDestinationStdout = "stdout"

// Pulse EMF config
pulseMetricNamespace = "AWS/APM"
pulseLogGroupNamePrefix = "/aws/apm/"
// AppSignals EMF config
appSignalsMetricNamespace = "AppSignals"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we omitting the AWS/ prefix? I think we should still do this as it is reserved

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spoke offline, this follows same pattern as container insights

appSignalsLogGroupNamePrefix = "/aws/appsignals/"
)

type emfExporter struct {
Expand Down Expand Up @@ -69,7 +69,7 @@ func newEmfExporter(config *Config, set exporter.CreateSettings) (*emfExporter,
config.Tags,
session,
cwlogs.WithEnabledContainerInsights(config.IsEnhancedContainerInsights()),
cwlogs.WithEnabledPulseApm(config.IsPulseApmEnabled()),
cwlogs.WithEnabledAppSignals(config.IsAppSignalsEnabled()),
)
collectorIdentifier, err := uuid.NewRandom()

Expand Down
12 changes: 6 additions & 6 deletions internal/aws/cwlogs/cwlog_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type UserAgentOption func(*UserAgentFlag)

type UserAgentFlag struct {
isEnhancedContainerInsights bool
isPulseApm bool
isAppSignals bool
}

func WithEnabledContainerInsights(flag bool) UserAgentOption {
Expand All @@ -52,9 +52,9 @@ func WithEnabledContainerInsights(flag bool) UserAgentOption {
}
}

func WithEnabledPulseApm(flag bool) UserAgentOption {
func WithEnabledAppSignals(flag bool) UserAgentOption {
return func(ua *UserAgentFlag) {
ua.isPulseApm = flag
ua.isAppSignals = flag
}
}

Expand All @@ -76,7 +76,7 @@ func NewClient(logger *zap.Logger, awsConfig *aws.Config, buildInfo component.Bu
// Loop through each option
option := &UserAgentFlag{
isEnhancedContainerInsights: false,
isPulseApm: false,
isAppSignals: false,
}
for _, opt := range opts {
opt(option)
Expand Down Expand Up @@ -229,8 +229,8 @@ func newCollectorUserAgentHandler(buildInfo component.BuildInfo, logGroupName st
extraStr = "EnhancedEKSContainerInsights"
case containerInsightsRegexPattern.MatchString(logGroupName):
extraStr = "ContainerInsights"
case userAgentFlag.isPulseApm:
extraStr = "Pulse"
case userAgentFlag.isAppSignals:
extraStr = "AppSignals"
}

fn := request.MakeAddToUserAgentHandler(buildInfo.Command, buildInfo.Version, extraStr)
Expand Down
22 changes: 11 additions & 11 deletions internal/aws/cwlogs/cwlog_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,10 +596,10 @@ func TestUserAgent(t *testing.T) {
"opentelemetry-collector-contrib/1.0",
},
{
"emptyLogGroupPulse",
"emptyLogGroupAppSignals",
component.BuildInfo{Command: "opentelemetry-collector-contrib", Version: "1.0"},
"",
WithEnabledPulseApm(false),
WithEnabledAppSignals(false),
"opentelemetry-collector-contrib/1.0",
},
{
Expand All @@ -610,10 +610,10 @@ func TestUserAgent(t *testing.T) {
"test-collector-contrib/1.0",
},
{
"buildInfoCommandUsedPulse",
"buildInfoCommandUsedAppSignals",
component.BuildInfo{Command: "test-collector-contrib", Version: "1.0"},
"",
WithEnabledPulseApm(false),
WithEnabledAppSignals(false),
"test-collector-contrib/1.0",
},
{
Expand Down Expand Up @@ -660,17 +660,17 @@ func TestUserAgent(t *testing.T) {
"opentelemetry-collector-contrib/1.0 (ContainerInsights)",
},
{
"validPulseEMFEnabled",
"validAppSignalsEMFEnabled",
component.BuildInfo{Command: "opentelemetry-collector-contrib", Version: "1.0"},
"/aws/apm",
WithEnabledPulseApm(true),
"opentelemetry-collector-contrib/1.0 (Pulse)",
"/aws/appsignals",
WithEnabledAppSignals(true),
"opentelemetry-collector-contrib/1.0 (AppSignals)",
},
{
"PulseEMFNotEnabled",
"AppSignalsEMFNotEnabled",
component.BuildInfo{Command: "opentelemetry-collector-contrib", Version: "1.0"},
"/aws/apm",
WithEnabledPulseApm(false),
"/aws/appsignals",
WithEnabledAppSignals(false),
"opentelemetry-collector-contrib/1.0",
},
}
Expand Down
Loading