From 7f0e9a45db2c40a57943ac9121ad246ea4523c72 Mon Sep 17 00:00:00 2001 From: Ben Carr Date: Fri, 15 Mar 2024 20:25:36 -0700 Subject: [PATCH 01/10] Add a new flag to read custom formatting from a file and add to the existing capitalizations slice --- semconvgen/generator.go | 55 ++++++++++++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/semconvgen/generator.go b/semconvgen/generator.go index ac1ac861..4deccc87 100644 --- a/semconvgen/generator.go +++ b/semconvgen/generator.go @@ -15,6 +15,7 @@ package main import ( + "bufio" "bytes" "errors" "fmt" @@ -48,6 +49,7 @@ func main() { flag.StringVarP(&cfg.outputFilename, "filename", "f", "", "Filename for templated output. If not specified 'basename(inputPath).go' will be used.") flag.StringVarP(&cfg.templateFilename, "template", "t", "template.j2", "Template filename") flag.StringVarP(&cfg.templateParameters, "parameters", "p", "", "List of key=value pairs separated by comma. These values are fed into the template as-is.") + flag.StringVarP(&cfg.capitalizationsPath, "capitalizations-path", "z", "", "Path to a file of newline separated capitalization strings.") flag.Parse() cfg, err := validateConfig(cfg) @@ -74,14 +76,15 @@ func main() { } type config struct { - inputPath string - outputPath string - outputFilename string - templateFilename string - templateParameters string - onlyType string - containerImage string - specVersion string + inputPath string + outputPath string + outputFilename string + templateFilename string + templateParameters string + onlyType string + containerImage string + specVersion string + capitalizationsPath string } func validateConfig(cfg config) (config, error) { @@ -126,6 +129,12 @@ func validateConfig(cfg config) (config, error) { cfg.templateFilename = path.Join(pwd, cfg.templateFilename) } + if cfg.capitalizationsPath != "" { + if _, err := os.Stat(cfg.capitalizationsPath); os.IsNotExist(err) { + return config{}, fmt.Errorf("capitalizations file does not exist: %w", err) + } + } + return cfg, nil } @@ -282,7 +291,7 @@ func checkoutSpecToDir(cfg config, toDir string) (doneFunc func(), err error) { return doneFunc, nil } -var capitalizations = []string{ +var staticCapitalizations = []string{ "ACL", "AIX", "AKS", @@ -379,6 +388,30 @@ var capitalizations = []string{ "OTel", } +func capitalizations(cfg config) ([]string, error) { + c := append([]string(nil), staticCapitalizations...) + + if cfg.capitalizationsPath != "" { + file, err := os.Open(cfg.capitalizationsPath) + if err != nil { + return nil, fmt.Errorf("unable to open capitalizations file: %w", err) + } + defer file.Close() + + scanner := bufio.NewScanner(file) + for scanner.Scan() { + capitalization := strings.TrimSpace(scanner.Text()) + c = append(c, capitalization) + } + + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("unable to read capitalizations file: %w", err) + } + } + + return c, nil +} + // These are not simple capitalization fixes, but require string replacement. // All occurrences of the key will be replaced with the corresponding value. var replacements = map[string]string{ @@ -394,6 +427,10 @@ func fixIdentifiers(cfg config) error { return fmt.Errorf("unable to read file: %w", err) } + capitalizations, err := capitalizations(cfg) + if err != nil { + return fmt.Errorf("unable to combine custom and static capitalizations: %w", err) + } caser := cases.Title(language.Und) for _, init := range capitalizations { // Match the title-cased capitalization target, asserting that its followed by From 65a6f4e5ad891b85a47c04278443168059be9224 Mon Sep 17 00:00:00 2001 From: Ben Carr Date: Sat, 16 Mar 2024 10:08:07 -0700 Subject: [PATCH 02/10] update semconvgen README to reflect the newly added flag --- semconvgen/README.md | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/semconvgen/README.md b/semconvgen/README.md index 3e422fb0..6590a165 100644 --- a/semconvgen/README.md +++ b/semconvgen/README.md @@ -22,13 +22,14 @@ semconvgen -i -t -o A full list of available options: -```txt - -c, --container string Container image ID (default "otel/semconvgen") - -f, --filename string Filename for templated output. If not specified 'basename(inputPath).go' will be used. - -i, --input string Path to semantic convention definition YAML. Should be a directory in the specification git repository. - --only string Process only semantic conventions of the specified type. {span, resource, event, metric_group, metric, units, scope, attribute_group} - -o, --output string Path to output target. Must be either an absolute path or relative to the repository root. If unspecified will output to a sub-directory with the name matching the version number specified via --specver flag. - -p, --parameters string List of key=value pairs separated by comma. These values are fed into the template as-is. - -s, --specver string Version of semantic convention to generate. Must be an existing version tag in the specification git repository. - -t, --template string Template filename (default "template.j2") -``` +````txt + -z, --capitalizations-path string Path to a file of newline separated capitalization strings. + -c, --container string Container image ID (default "otel/semconvgen") + -f, --filename string Filename for templated output. If not specified 'basename(inputPath).go' will be used. + -i, --input string Path to semantic convention definition YAML. Should be a directory in the specification git repository. + --only string Process only semantic conventions of the specified type. {span, resource, event, metric_group, metric, units, scope, attribute_group} + -o, --output string Path to output target. Must be either an absolute path or relative to the repository root. If unspecified will output to a sub-directory with the name matching the version number specified via --specver flag. + -p, --parameters string List of key=value pairs separated by comma. These values are fed into the template as-is. + -s, --specver string Version of semantic convention to generate. Must be an existing version tag in the specification git repository. + -t, --template string Template filename (default "template.j2")``` +```` From b2e1e9defff206073db93bf56dd787c9fd3100de Mon Sep 17 00:00:00 2001 From: Ben Carr Date: Sat, 16 Mar 2024 10:09:28 -0700 Subject: [PATCH 03/10] add new semconvgen flag to CHANGELOG --- ...mconvgen-flag-for-custom-capitalizations.yaml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 .chloggen/528-semconvgen-flag-for-custom-capitalizations.yaml diff --git a/.chloggen/528-semconvgen-flag-for-custom-capitalizations.yaml b/.chloggen/528-semconvgen-flag-for-custom-capitalizations.yaml new file mode 100644 index 00000000..6aa93bf5 --- /dev/null +++ b/.chloggen/528-semconvgen-flag-for-custom-capitalizations.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. crosslink) +component: semconvgen + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Add `--capitalizations-path` to allow users to add additional strings to the static capitalizations slice in generator.go" + +# One or more tracking issues related to the change +issues: [528] + +# (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: From 09939551184dd5eba3cf8a76206ae42c068c1590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 10 Apr 2024 15:57:40 +0200 Subject: [PATCH 04/10] Update semconvgen/README.md Co-authored-by: Damien Mathieu <42@dmathieu.com> --- semconvgen/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/semconvgen/README.md b/semconvgen/README.md index 6590a165..dbfe7365 100644 --- a/semconvgen/README.md +++ b/semconvgen/README.md @@ -23,7 +23,7 @@ semconvgen -i -t -o A full list of available options: ````txt - -z, --capitalizations-path string Path to a file of newline separated capitalization strings. + -z, --capitalizations-path string Path to a file of newline separated capitalization strings that will be appended to the default list. -c, --container string Container image ID (default "otel/semconvgen") -f, --filename string Filename for templated output. If not specified 'basename(inputPath).go' will be used. -i, --input string Path to semantic convention definition YAML. Should be a directory in the specification git repository. From 798c8865d650bcb93f95b637146be2e23d550950 Mon Sep 17 00:00:00 2001 From: Ben Carr Date: Wed, 10 Apr 2024 10:59:28 -0700 Subject: [PATCH 05/10] Clarify usage information for capitalization-path flag --- semconvgen/README.md | 2 +- semconvgen/generator.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/semconvgen/README.md b/semconvgen/README.md index dbfe7365..645dae84 100644 --- a/semconvgen/README.md +++ b/semconvgen/README.md @@ -23,7 +23,7 @@ semconvgen -i -t -o A full list of available options: ````txt - -z, --capitalizations-path string Path to a file of newline separated capitalization strings that will be appended to the default list. + -z, --capitalizations-path string Path to a file containing additional newline-separated capitalization strings. -c, --container string Container image ID (default "otel/semconvgen") -f, --filename string Filename for templated output. If not specified 'basename(inputPath).go' will be used. -i, --input string Path to semantic convention definition YAML. Should be a directory in the specification git repository. diff --git a/semconvgen/generator.go b/semconvgen/generator.go index 4deccc87..e55d3b26 100644 --- a/semconvgen/generator.go +++ b/semconvgen/generator.go @@ -49,7 +49,7 @@ func main() { flag.StringVarP(&cfg.outputFilename, "filename", "f", "", "Filename for templated output. If not specified 'basename(inputPath).go' will be used.") flag.StringVarP(&cfg.templateFilename, "template", "t", "template.j2", "Template filename") flag.StringVarP(&cfg.templateParameters, "parameters", "p", "", "List of key=value pairs separated by comma. These values are fed into the template as-is.") - flag.StringVarP(&cfg.capitalizationsPath, "capitalizations-path", "z", "", "Path to a file of newline separated capitalization strings.") + flag.StringVarP(&cfg.capitalizationsPath, "capitalizations-path", "z", "", "Path to a file containing additional newline-separated capitalization strings.") flag.Parse() cfg, err := validateConfig(cfg) From e9ff3528d00b84dfbfec4b34ff23d4eeffc7c1d4 Mon Sep 17 00:00:00 2001 From: Ben Carr Date: Wed, 10 Apr 2024 11:55:28 -0700 Subject: [PATCH 06/10] replace `cfg config` with the `capitalizationsPath []string` as argument to the capitalizations function --- semconvgen/generator.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/semconvgen/generator.go b/semconvgen/generator.go index e55d3b26..3b54b58b 100644 --- a/semconvgen/generator.go +++ b/semconvgen/generator.go @@ -388,11 +388,12 @@ var staticCapitalizations = []string{ "OTel", } -func capitalizations(cfg config) ([]string, error) { +func capitalizations(capitalizationsPath string) ([]string, error) { c := append([]string(nil), staticCapitalizations...) - if cfg.capitalizationsPath != "" { - file, err := os.Open(cfg.capitalizationsPath) + if capitalizationsPath != "" { + // #nosec G304 + file, err := os.Open(capitalizationsPath) if err != nil { return nil, fmt.Errorf("unable to open capitalizations file: %w", err) } @@ -427,7 +428,7 @@ func fixIdentifiers(cfg config) error { return fmt.Errorf("unable to read file: %w", err) } - capitalizations, err := capitalizations(cfg) + capitalizations, err := capitalizations(cfg.capitalizationsPath) if err != nil { return fmt.Errorf("unable to combine custom and static capitalizations: %w", err) } From a01129a44c8894ba97d776113c9212c251523763 Mon Sep 17 00:00:00 2001 From: Ben Carr Date: Thu, 11 Apr 2024 12:43:52 -0700 Subject: [PATCH 07/10] Adds unit tests for the custom capitalizations flag --- semconvgen/generator_test.go | 57 ++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 semconvgen/generator_test.go diff --git a/semconvgen/generator_test.go b/semconvgen/generator_test.go new file mode 100644 index 00000000..5b814b55 --- /dev/null +++ b/semconvgen/generator_test.go @@ -0,0 +1,57 @@ +package main + +import ( + "os" + "reflect" + "testing" +) + +func TestCapitalizations(t *testing.T) { + tests := []struct { + name string + capitalizations string + expected []string + }{ + { + name: "No additional capitalizations", + capitalizations: "", + expected: staticCapitalizations, + }, + { + name: "Some additional capitalizations", + capitalizations: "ASPNETCore\nJVM", + expected: append(staticCapitalizations, "ASPNETCore", "JVM"), + }, + { + name: "Wrong separator for capitalizations", + capitalizations: "ASPNETCore,JVM", + expected: append(staticCapitalizations, "ASPNETCore,JVM"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpfile, err := os.CreateTemp("", "test") + if err != nil { + t.Fatal(err) + } + defer os.Remove(tmpfile.Name()) + + if _, err = tmpfile.Write([]byte(tt.capitalizations)); err != nil { + t.Fatal(err) + } + if err = tmpfile.Close(); err != nil { + t.Fatal(err) + } + + customCapitalizations, err := capitalizations(tmpfile.Name()) + if err != nil { + t.Fatal(err) + } + + if !reflect.DeepEqual(customCapitalizations, tt.expected) { + t.Errorf("customCapitalizations() = %v, want %v", customCapitalizations, tt.expected) + } + }) + } +} From e2256e0414f4cb6a209835f7d2e52bddc03eaca3 Mon Sep 17 00:00:00 2001 From: Ben Carr Date: Thu, 11 Apr 2024 12:51:40 -0700 Subject: [PATCH 08/10] Adds justification for suppressing the G304 security rule --- semconvgen/generator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/semconvgen/generator.go b/semconvgen/generator.go index 3b54b58b..e9569f77 100644 --- a/semconvgen/generator.go +++ b/semconvgen/generator.go @@ -392,7 +392,7 @@ func capitalizations(capitalizationsPath string) ([]string, error) { c := append([]string(nil), staticCapitalizations...) if capitalizationsPath != "" { - // #nosec G304 + // #nosec G304 -- We expect the file path to be provided by the user. file, err := os.Open(capitalizationsPath) if err != nil { return nil, fmt.Errorf("unable to open capitalizations file: %w", err) From 617ffa6d993e6d62ac23958dc6bee45b5f1175a7 Mon Sep 17 00:00:00 2001 From: Ben Carr Date: Thu, 11 Apr 2024 16:19:22 -0700 Subject: [PATCH 09/10] remove empty strings from the slice returned by the capitalizations() function --- semconvgen/generator.go | 3 +++ semconvgen/generator_test.go | 24 ++++++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/semconvgen/generator.go b/semconvgen/generator.go index e9569f77..4ff1c656 100644 --- a/semconvgen/generator.go +++ b/semconvgen/generator.go @@ -402,6 +402,9 @@ func capitalizations(capitalizationsPath string) ([]string, error) { scanner := bufio.NewScanner(file) for scanner.Scan() { capitalization := strings.TrimSpace(scanner.Text()) + if capitalization == "" { + continue + } c = append(c, capitalization) } diff --git a/semconvgen/generator_test.go b/semconvgen/generator_test.go index 5b814b55..1e26e00a 100644 --- a/semconvgen/generator_test.go +++ b/semconvgen/generator_test.go @@ -10,22 +10,34 @@ func TestCapitalizations(t *testing.T) { tests := []struct { name string capitalizations string - expected []string + want []string }{ { name: "No additional capitalizations", capitalizations: "", - expected: staticCapitalizations, + want: staticCapitalizations, }, { name: "Some additional capitalizations", capitalizations: "ASPNETCore\nJVM", - expected: append(staticCapitalizations, "ASPNETCore", "JVM"), + want: append(staticCapitalizations, "ASPNETCore", "JVM"), }, { name: "Wrong separator for capitalizations", capitalizations: "ASPNETCore,JVM", - expected: append(staticCapitalizations, "ASPNETCore,JVM"), + want: append(staticCapitalizations, "ASPNETCore,JVM"), + }, + { + name: "Copius amounts of whitespace", + capitalizations: ` + + ASPNETCore + + JVM + + + `, + want: append(staticCapitalizations, "ASPNETCore", "JVM"), }, } @@ -49,8 +61,8 @@ func TestCapitalizations(t *testing.T) { t.Fatal(err) } - if !reflect.DeepEqual(customCapitalizations, tt.expected) { - t.Errorf("customCapitalizations() = %v, want %v", customCapitalizations, tt.expected) + if !reflect.DeepEqual(customCapitalizations, tt.want) { + t.Errorf("capitalizations() = %v, want %v", customCapitalizations, tt.want) } }) } From 2e3b1fea4587ca7a2410a41d217525fbfcf96156 Mon Sep 17 00:00:00 2001 From: Ben Carr Date: Sat, 13 Apr 2024 09:00:22 -0700 Subject: [PATCH 10/10] add license header to generator_test.go --- semconvgen/generator_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/semconvgen/generator_test.go b/semconvgen/generator_test.go index 1e26e00a..cfa1cd45 100644 --- a/semconvgen/generator_test.go +++ b/semconvgen/generator_test.go @@ -1,3 +1,17 @@ +// Copyright The OpenTelemetry 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 (