Analytics emits error if environment is missing
Closed this issue · 6 comments
rockspore commented
Below is the captured log:
2021-02-02T01:09:54.036Z ERROR analytics/manager.go:326 invalid record {1612228193128 1612228193128 1612228193128 1612228193128 1612228193128 1612228193128 1612228193132 1612228193132 APIAnalytics httpbin.default.svc.cluster.local /headers /headers GET curl/7.35.0 0 403 apigee_org envoy 2705f305-4616-4948-a8a7-ffc609b318c7}: 1 error occurred:
* missing Environment
github.com/apigee/apigee-remote-service-golib/analytics.(*manager).SendRecords
/go/pkg/mod/github.com/apigee/apigee-remote-service-golib@v1.4.1-0.20210127172319-6ac528b724ab/analytics/manager.go:326
github.com/apigee/apigee-remote-service-envoy/server.(*AccessLogServer).handleHTTPLogs
/app/server/accesslog.go:124
github.com/apigee/apigee-remote-service-envoy/server.(*AccessLogServer).StreamAccessLogs
/app/server/accesslog.go:66
github.com/envoyproxy/go-control-plane/envoy/service/accesslog/v3._AccessLogService_StreamAccessLogs_Handler
/go/pkg/mod/github.com/envoyproxy/go-control-plane@v0.9.7/envoy/service/accesslog/v3/als.pb.go:641
github.com/grpc-ecosystem/go-grpc-prometheus.(*ServerMetrics).StreamServerInterceptor.func1
/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-prometheus@v1.2.0/server_metrics.go:121
google.golang.org/grpc.(*Server).processStreamingRPC
/go/pkg/mod/google.golang.org/grpc@v1.33.2/server.go:1464
google.golang.org/grpc.(*Server).handleStream
/go/pkg/mod/google.golang.org/grpc@v1.33.2/server.go:1537
google.golang.org/grpc.(*Server).serveStreams.func1.2
/go/pkg/mod/google.golang.org/grpc@v1.33.2/server.go:871
theganyo commented
That is an error. What are you proposing?
rockspore commented
I wondered if it was okay that such records would then be discarded and never sent to analytics. Fundamentally, this is indeed an error.
theganyo commented
It needs to be flagged for users, but it could be a warning instead of error.
rockspore commented
Based on the fact that this is not the fault of the adapter under the multi-env setup, I prefer to change it to a warning.
theganyo commented
Makes sense. It really shouldn't happen under normal use, though, so probably not less than warning.
rockspore commented
On second thought, it doesn't seem worthwhile to break the record validation logic. There is no mechanism of sending this record anyway.