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

[chore] Check component API #25855

Merged
merged 5 commits into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 2 additions & 0 deletions .github/workflows/build-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,8 @@ jobs:
run: make checkdoc
- name: CheckMetadata
run: make checkmetadata
- name: CheckApi
run: make checkapi
- name: Porto
run: |
make -j2 goporto
Expand Down
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,10 @@ checkdoc: $(CHECKFILE)
checkmetadata: $(CHECKFILE)
$(CHECKFILE) --project-path $(CURDIR) --component-rel-path $(COMP_REL_PATH) --module-name $(MOD_NAME) --file-name "metadata.yaml"

.PHONY: checkapi
checkapi:
$(GOCMD) run cmd/checkapi/main.go .

.PHONY: all-checklinks
all-checklinks:
$(MAKE) $(FOR_GROUP_TARGET) TARGET="checklinks"
Expand Down
44 changes: 44 additions & 0 deletions cmd/checkapi/allowlist.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
connector/servicegraphconnector
exporter/alibabacloudlogserviceexporter
exporter/awscloudwatchlogsexporter
exporter/awskinesisexporter
exporter/awss3exporter
exporter/azuremonitorexporter
exporter/coralogixexporter
exporter/datasetexporter
exporter/dynatraceexporter
exporter/elasticsearchexporter
exporter/f5cloudexporter
exporter/jaegerexporter
exporter/kafkaexporter
exporter/loadbalancingexporter
exporter/logzioexporter
exporter/lokiexporter
exporter/pulsarexporter
exporter/sentryexporter
exporter/sumologicexporter
extension/observer/ecsobserver
extension/observer
extension/observer/k8sobserver
processor/groupbyattrsprocessor
processor/groupbytraceprocessor
processor/probabilisticsamplerprocessor
processor/servicegraphprocessor
processor/tailsamplingprocessor
receiver/carbonreceiver
receiver/collectdreceiver
receiver/dockerstatsreceiver
receiver/fluentforwardreceiver
receiver/jaegerreceiver
receiver/journaldreceiver
receiver/k8sobjectsreceiver
receiver/kafkareceiver
receiver/mongodbatlasreceiver
receiver/mongodbreceiver
receiver/mysqlreceiver
receiver/nsxtreceiver
receiver/otlpjsonfilereceiver
receiver/podmanreceiver
receiver/pulsarreceiver
receiver/statsdreceiver
receiver/windowseventlogreceiver
240 changes: 240 additions & 0 deletions cmd/checkapi/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,240 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package main

import (
"errors"
"flag"
"fmt"
"go/ast"
"go/parser"
"go/token"
"io/fs"
"log"
"os"
"path/filepath"
"sort"
"strings"
)

func main() {
atoulme marked this conversation as resolved.
Show resolved Hide resolved
folder := flag.String("folder", ".", "folder investigated for modules")
allowlistFilePath := flag.String("allowlist", "cmd/checkapi/allowlist.txt", "path to a file containing an allowlist of paths to ignore")
flag.Parse()
if err := run(*folder, *allowlistFilePath); err != nil {
log.Fatal(err)
}
}

type function struct {
Name string `json:"name"`
Receiver string `json:"receiver"`
ReturnTypes []string `json:"return_types,omitempty"`
ParamTypes []string `json:"param_types,omitempty"`
}

type api struct {
Values []string `json:"values,omitempty"`
Structs []string `json:"structs,omitempty"`
Functions []*function `json:"functions,omitempty"`
}

func run(folder string, allowlistFilePath string) error {
allowlistData, err := os.ReadFile(allowlistFilePath)
if err != nil {
return err
}
allowlist := strings.Split(string(allowlistData), "\n")
var errs []error
err = filepath.Walk(folder, func(path string, info fs.FileInfo, err error) error {
if info.Name() == "go.mod" {
base := filepath.Dir(path)
relativeBase, err := filepath.Rel(folder, base)
if err != nil {
return err
}
componentType := strings.Split(relativeBase, string(os.PathSeparator))[0]
if componentType != "receiver" && componentType != "processor" && componentType != "exporter" && componentType != "connector" && componentType != "extension" {
return nil
}
atoulme marked this conversation as resolved.
Show resolved Hide resolved

for _, a := range allowlist {
if a == relativeBase {
fmt.Printf("Ignoring %s per allowlist\n", base)
return nil
}
}
if err := walkFolder(base, componentType); err != nil {
errs = append(errs, err)
}
}
return nil
})
if err != nil {
return err
}
if len(errs) > 0 {
return errors.Join(errs...)
}
return nil
}

func walkFolder(folder string, componentType string) error {
result := &api{}
set := token.NewFileSet()
packs, err := parser.ParseDir(set, folder, nil, 0)
if err != nil {
return err
}

for _, pack := range packs {
for _, f := range pack.Files {
for _, d := range f.Decls {
if str, isStr := d.(*ast.GenDecl); isStr {
for _, s := range str.Specs {
if values, ok := s.(*ast.ValueSpec); ok {
for _, v := range values.Names {
if v.IsExported() {
result.Values = append(result.Values, v.Name)
}
}
}
if t, ok := s.(*ast.TypeSpec); ok {
if t.Name.IsExported() {
result.Structs = append(result.Structs, t.Name.String())
}
}

}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to reduce the level of nesting within this section?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how I'd do that. I could move code around into a separate function but I'm not sure it would help with clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK I am trying something, let me know if that looks good to you

if fn, isFn := d.(*ast.FuncDecl); isFn {
if !fn.Name.IsExported() {
continue
}
exported := false
receiver := ""
if fn.Recv.NumFields() == 0 && !strings.HasPrefix(fn.Name.String(), "Test") && !strings.HasPrefix(fn.Name.String(), "Benchmark") {
exported = true
}
if fn.Recv.NumFields() > 0 {

for _, t := range fn.Recv.List {
for _, n := range t.Names {
exported = exported || n.IsExported()
if n.IsExported() {
receiver = n.Name
}
}
}
}
if exported {
var returnTypes []string
if fn.Type.Results.NumFields() > 0 {
for _, r := range fn.Type.Results.List {
returnTypes = append(returnTypes, exprToString(r.Type))
}
}
var params []string
if fn.Type.Params.NumFields() > 0 {
for _, r := range fn.Type.Params.List {
params = append(params, exprToString(r.Type))
}
}
f := &function{
Name: fn.Name.Name,
Receiver: receiver,
ParamTypes: params,
ReturnTypes: returnTypes,
}
result.Functions = append(result.Functions, f)
}
}
}
}
}
sort.Strings(result.Structs)
sort.Strings(result.Values)
sort.Slice(result.Functions, func(i int, j int) bool {
return strings.Compare(result.Functions[i].Name, result.Functions[j].Name) < 0
})
fnNames := make([]string, len(result.Functions))
for i, fn := range result.Functions {
fnNames[i] = fn.Name
}
if len(result.Structs) == 0 && len(result.Values) == 0 && len(result.Functions) == 0 {
return nil
}

if len(result.Functions) > 1 {
return fmt.Errorf("%s has more than one function: %q", folder, strings.Join(fnNames, ","))
}
if len(result.Functions) == 0 {
return fmt.Errorf("%s has no functions defined", folder)
}
newFactoryFn := result.Functions[0]
if newFactoryFn.Name != "NewFactory" {
return fmt.Errorf("%s does not define a NewFactory function", folder)
}
if newFactoryFn.Receiver != "" {
return fmt.Errorf("%s associated NewFactory with a receiver type", folder)
}
if len(newFactoryFn.ReturnTypes) != 1 {
return fmt.Errorf("%s NewFactory function returns more than one result", folder)
}
returnType := newFactoryFn.ReturnTypes[0]

if returnType != fmt.Sprintf("%s.Factory", componentType) {
return fmt.Errorf("%s NewFactory function does not return a valid type: %s, expected %s.Factory", folder, returnType, componentType)
}
return nil
}

func exprToString(expr ast.Expr) string {
switch e := expr.(type) {
case *ast.MapType:
return fmt.Sprintf("map[%s]%s", exprToString(e.Key), exprToString(e.Value))
case *ast.ArrayType:
return fmt.Sprintf("[%s]%s", exprToString(e.Len), exprToString(e.Elt))
case *ast.StructType:
var fields []string
for _, f := range e.Fields.List {
fields = append(fields, exprToString(f.Type))
}
return fmt.Sprintf("{%s}", strings.Join(fields, ","))
case *ast.InterfaceType:
var methods []string
for _, f := range e.Methods.List {
methods = append(methods, "func "+exprToString(f.Type))
}
return fmt.Sprintf("{%s}", strings.Join(methods, ","))
case *ast.ChanType:
return fmt.Sprintf("chan(%s)", exprToString(e.Value))
case *ast.FuncType:
var results []string
for _, r := range e.Results.List {
results = append(results, exprToString(r.Type))
}
var params []string
for _, r := range e.Params.List {
params = append(params, exprToString(r.Type))
}
return fmt.Sprintf("func(%s) %s", strings.Join(params, ","), strings.Join(results, ","))
case *ast.SelectorExpr:
return fmt.Sprintf("%s.%s", exprToString(e.X), e.Sel.Name)
case *ast.Ident:
return e.Name
case nil:
return ""
case *ast.StarExpr:
return fmt.Sprintf("*%s", exprToString(e.X))
case *ast.Ellipsis:
return fmt.Sprintf("%s...", exprToString(e.Elt))
case *ast.IndexExpr:
return fmt.Sprintf("%s[%s]", exprToString(e.X), exprToString(e.Index))
case *ast.BasicLit:
return e.Value
default:
panic(fmt.Sprintf("Unsupported expr type: %#v", expr))
}
}