Weaver #5898
Open
Weaver #5898
+342
−208
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5898 +/- ##
=====================================
Coverage 82.2% 82.2%
=====================================
Files 273 273
Lines 23744 23744
=====================================
Hits 19537 19537
Misses 3858 3858
Partials 349 349 |
MrAlias
force-pushed
the
weaver
branch
2 times, most recently
from
a9d4f9e to
dab26e9
Compare
16 tasks
MrAlias
force-pushed
the
weaver
branch
2 times, most recently
from
82b3ba8 to
e94b8b5
Compare
|
It looks like we have parity for the exported types when generating package main
import (
"fmt"
"go/ast"
"go/parser"
"go/token"
"log"
"os"
)
func main() {
if len(os.Args) < 2 {
log.Fatal("Please provide a package path")
}
pkgPath := os.Args[1]
fset := token.NewFileSet()
pkgs, err := parser.ParseDir(fset, pkgPath, nil, 0)
if err != nil {
log.Fatal(err)
}
for _, pkg := range pkgs {
for _, file := range pkg.Files {
for _, decl := range file.Decls {
if funcDecl, ok := decl.(*ast.FuncDecl); ok {
fmt.Println(funcDecl.Name.Name)
} else if genDecl, ok := decl.(*ast.GenDecl); ok {
if genDecl.Tok == token.CONST || genDecl.Tok == token.VAR {
for _, spec := range genDecl.Specs {
if valueSpec, ok := spec.(*ast.ValueSpec); ok {
for _, name := range valueSpec.Names {
fmt.Println(name.Name)
}
}
}
}
}
}
}
}
}Using that output I can match the differences between this branch ( package main
import (
"bufio"
"fmt"
"os"
"sort"
"strings"
)
func readIdentifiers(filename string) (map[string]string, error) {
file, err := os.Open(filename)
if err != nil {
return nil, err
}
defer file.Close()
identifiers := make(map[string]string)
scanner := bufio.NewScanner(file)
for scanner.Scan() {
line := scanner.Text()
identifiers[line] = line
}
if err := scanner.Err(); err != nil {
return nil, err
}
return identifiers, nil
}
func findDifferences(file1, file2 string) error {
ids1, err := readIdentifiers(file1)
if err != nil {
return err
}
ids2, err := readIdentifiers(file2)
if err != nil {
return err
}
lower1 := make(map[string]string)
lower2 := make(map[string]string)
for id := range ids1 {
lower1[strings.ToLower(id)] = id
}
for id := range ids2 {
lower2[strings.ToLower(id)] = id
}
caseInsensitiveDiff := make([]string, 0)
caseSensitiveDiff := make([]string, 0)
// Find case-insensitive differences
for lowerID, originalID := range lower1 {
if _, exists := lower2[lowerID]; !exists {
caseInsensitiveDiff = append(caseInsensitiveDiff, fmt.Sprintf("%s (from %s)", originalID, file1))
}
}
for lowerID, originalID := range lower2 {
if _, exists := lower1[lowerID]; !exists {
caseInsensitiveDiff = append(caseInsensitiveDiff, fmt.Sprintf("%s (from %s)", originalID, file2))
}
}
// Find case-sensitive differences
for lowerID, id1 := range lower1 {
if id2, exists := lower2[lowerID]; exists && id1 != id2 {
caseSensitiveDiff = append(caseSensitiveDiff, fmt.Sprintf("%s <-> %s", id1, id2))
}
}
sort.Strings(caseInsensitiveDiff)
sort.Strings(caseSensitiveDiff)
fmt.Println("Case Insensitive Difference (Unique Identifiers):")
for _, id := range caseInsensitiveDiff {
fmt.Println(id)
}
fmt.Println("\nCase Sensitive Difference (Mismatched Casing):")
for _, diff := range caseSensitiveDiff {
fmt.Println(diff)
}
return nil
}
func main() {
if len(os.Args) < 3 {
fmt.Println("Usage: go run main.go <file1> <file2>")
os.Exit(1)
}
file1 := os.Args[1]
file2 := os.Args[2]
if err := findDifferences(file1, file2); err != nil {
fmt.Println("Error:", err)
os.Exit(1)
}
}This results in: This shows that the only exported differences are in acronym/initialism naming. Reviewing these changes, they all seem to be ones we want to make. This only evaluates the exported declarations. There are still documentation changes that are included. Mostly this is unavoidable based on the new format of the semconv models. |
Replace the templates and configuration for semconvgen in the semconv package to support weaver instead.
MrAlias
force-pushed
the
weaver
branch
2 times, most recently
from
98bd378 to
c2c9fd7
Compare
MrAlias
added
Skip Changelog
Do not reference the dropped otel/semconvgen image.
This reverts commit 66523cb.
MrAlias
requested review from
XSAM,
dashpole,
pellared and
dmathieu
as code owners
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Builds off of #5793
Resolve #5668
This migrates the generation of our semconv packages from using the
semconvgentooling to the newweaverproject.The configuration and templating has been added in a way to generate as close as we can to what the
semconvgentooling already generated. There are notable differences:Acronym/Initialism Fixes
As metioned here, the evaluated exported output of regenerating the
semconv/v1.27.0package resulted in the following changes:weaversemconvgenAn audit of these changes leads to the conclusion that they are appropriate fixes to things that were mis-named.
Doc changes
Also mentioned here, there documentation changes that are included. Mostly this is unavoidable based on the new format of the semconv models, and effort has spent ensuring nothing substantive is lost.
See the reverted commit 66523cb for the details of how the
semconv/v1.27.0changes.Next Steps
This PR has been paired down to migrate tooling. The next steps are to generate
semconv/v1.28.0with desired changes (i.e. maybe we don't generateASPCoreNetattrs(?)). From there the missing semconv packages will be generated.semconv/v1.28.0#6226semconv/v1.30.0#6227