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

Add support for all CLP FFI functionality and refactor existing code: #3

Merged
merged 27 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
fba8f62
Large refactor and update
davidlion Jun 6, 2023
9253255
Reintroduce __cplusplus ifdefs to headers so the correct names are in…
davidlion Apr 10, 2024
33647d0
Apply suggestions from code review
davidlion Apr 25, 2024
2fd3f15
Apply suggestions from code review
davidlion Apr 25, 2024
c4abd24
Improve writing
davidlion Apr 25, 2024
50bec08
Fix extern C header ifdef to not wrap include statements.
davidlion Apr 25, 2024
98176cb
Add macos back in workflow
davidlion Apr 25, 2024
439f948
LogTypes.hpp -> types.hpp
davidlion Apr 25, 2024
a439133
Update cpp/src/ffi_go/ir/decoder.cpp
davidlion Apr 25, 2024
67003c2
Update cpp/src/ffi_go/ir/encoder.cpp
LinZhihao-723 Apr 25, 2024
2d570a8
Adding `typename` for dependent names for disambiguration purpose.
LinZhihao-723 Apr 25, 2024
edeab0d
Refactor c++ and add missing null checks.
davidlion Apr 25, 2024
99e9cea
Rename LogEvent to LogEventStorage.
davidlion Apr 25, 2024
06ea187
Apply suggestions from code review
LinZhihao-723 Apr 29, 2024
00b7904
Apply suggestions from code review
davidlion Apr 29, 2024
c6b9ec5
Refactor...
LinZhihao-723 Apr 29, 2024
8f7802c
Using api decoration.
LinZhihao-723 Apr 29, 2024
bc6fca9
Apply clang-tidy suggestions on src/ffi_go/ir/*
LinZhihao-723 Apr 29, 2024
3d06c36
Apply clang-tidy suggestions on regular files
LinZhihao-723 Apr 29, 2024
72e48bc
Update github workflow to specify Go versions.
LinZhihao-723 Apr 30, 2024
89139e0
Workflow debugging.
davidlion May 8, 2024
ef2eb59
Add api_decoration header.
davidlion May 8, 2024
85dab75
Fix linting.
davidlion May 8, 2024
b18618b
Fix workflows hopefully.
davidlion May 8, 2024
ed428c7
Removed confusing comment.
davidlion May 9, 2024
0dd51a2
Add darwin arm64 lib
LinZhihao-723 May 9, 2024
49d61fb
Added helper function to reader to perform a wildcard query with the …
davidlion May 9, 2024
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
40 changes: 40 additions & 0 deletions .github/ISSUE_TEMPLATE/bug-report.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
name: "Bug Report"
description: Report software deficiencies
labels: ["bug"]
body:
- type: markdown
attributes:
value: |
Use this form to report any functional or performance bugs you've found in the software.

Be sure to check if your [issue](https://github.com/y-scope/clp-ffi-go/issues) has already been reported.

- type: textarea
attributes:
label: Bug
description: "Describe what's wrong and if applicable, what you expected instead."
validations:
required: true

- type: input
attributes:
label: clp-ffi-go version
description: "The release version number or development commit hash that has the bug."
placeholder: "Version number or commit hash"
validations:
required: true

- type: textarea
attributes:
label: Environment
description: "The environment in which you're running/using clp-ffi-go."
placeholder: "OS version, docker version, etc."
validations:
required: true

- type: textarea
attributes:
label: Reproduction steps
description: "List each step required to reproduce the bug."
validations:
required: true
1 change: 1 addition & 0 deletions .github/ISSUE_TEMPLATE/config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
blank_issues_enabled: true
23 changes: 23 additions & 0 deletions .github/ISSUE_TEMPLATE/feature-request.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: "Feature/Change Request"
description: Request a feature or change
labels: ["enhancement"]
body:
- type: markdown
attributes:
value: |
Use this form to request a feature/change in the software, or the project as a whole.

- type: textarea
attributes:
label: Request
description: "Describe your request and why it's important."
validations:
required: true

- type: textarea
attributes:
label: Possible implementation
description: "Describe any implementations you have in mind."
validations:
required: true

9 changes: 9 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# References
<!-- Any issues or pull requests relevant to this pull request -->

# Description
<!-- Describe what this request will change/fix and provide any details necessary for reviewers -->

# Validation performed
<!-- What tests and validation you performed on the change -->

61 changes: 61 additions & 0 deletions .github/workflows/build.yml
davidlion marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
name: Build

on:
pull_request:
push:
workflow_call:

jobs:
prebuilt-test:
strategy:
matrix:
# os: [macos-latest, ubuntu-latest]
os: [ubuntu-latest]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v3
with:
submodules: recursive

- uses: actions/setup-go@v4

- run: go build ./...

- run: go test ./...

build-lint-test:
strategy:
matrix:
# os: [macos-latest, ubuntu-latest]
os: [ubuntu-latest]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v3
with:
submodules: recursive

- uses: actions/setup-go@v4

- if: ${{ 'macos-latest' == matrix.os }}
run: |
brew update
brew install llvm

- name: Remove repo's generated c++ libraries and go code
run: |
rm ./lib/* ./**/*_string.go

- run: |
go install mvdan.cc/gofumpt@latest
go install github.com/segmentio/golines@latest
go install golang.org/x/tools/cmd/stringer@latest

- run: go generate ./...

- run: |
diff="$(golines -m 100 -t 4 --base-formatter='gofumpt' --dry-run .)"
if [[ -n "$diff" ]]; then echo "$diff"; exit 1; fi

# - run: cmake -S ./cpp -B ./cpp/build -DCMAKE_EXPORT_COMPILE_COMMANDS=1

- run: go test ./...
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
build/
cpp/.cache/
cpp/clp/
**/compile_commands.json
3 changes: 0 additions & 3 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,3 +0,0 @@
[submodule "clp"]
path = cpp/clp
url = [email protected]:y-scope/clp.git
53 changes: 9 additions & 44 deletions BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,53 +1,18 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@rules_cc//cc:defs.bzl", "cc_library")

cc_library(
name = "libclp_ffi",
srcs = select({
"@io_bazel_rules_go//go/platform:android_amd64": [
"lib/libclp_ffi_linux_amd64.so",
],
"@io_bazel_rules_go//go/platform:android_arm64": [
"lib/libclp_ffi_linux_arm64.so",
],
"@io_bazel_rules_go//go/platform:darwin_amd64": [
"lib/libclp_ffi_darwin_amd64.so",
],
"@io_bazel_rules_go//go/platform:darwin_arm64": [
"lib/libclp_ffi_darwin_arm64.so",
],
"@io_bazel_rules_go//go/platform:ios_amd64": [
"lib/libclp_ffi_darwin_amd64.so",
],
"@io_bazel_rules_go//go/platform:ios_arm64": [
"lib/libclp_ffi_darwin_arm64.so",
],
"@io_bazel_rules_go//go/platform:linux_amd64": [
"lib/libclp_ffi_linux_amd64.so",
],
"@io_bazel_rules_go//go/platform:linux_arm64": [
"lib/libclp_ffi_linux_arm64.so",
],
"//conditions:default": [],
}),
hdrs = glob([
"cpp/src/**/*.h",
]),
srcs = glob(["cpp/src/ffi_go/**"]) + [
],
hdrs = glob(["cpp/src/ffi_go/**/*.h"]),
includes = [
"cpp/src",
],
visibility = ["//visibility:public"],
)

go_library(
name = "clp-ffi-go",
srcs = ["generate.go"],
importpath = "github.com/y-scope/clp-ffi-go",
visibility = ["//visibility:public"],
)

alias(
name = "go_default_library",
actual = ":clp-ffi-go",
deps = [
"@com_github_y_scope_clp//:libclp_core",
],
copts = [
"-std=c++20",
],
visibility = ["//visibility:public"],
)
3 changes: 2 additions & 1 deletion LICENSE
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

Apache License
Version 2.0, January 2004
http://www.apache.org/licenses/
Expand Down Expand Up @@ -186,7 +187,7 @@
same "printed page" as the copyright notice for easier
identification within third-party archives.

Copyright 2022 YScope Inc.
Copyright [yyyy] [name of copyright owner]

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down
43 changes: 24 additions & 19 deletions README.rst
davidlion marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Getting started
To add the module to your project run: ``go get github.com/y-scope/clp-ffi-go``

Here's an example showing how to decode each log event containing "ERROR" from
a CLP IR stream.
a CLP IR byte stream.

.. code:: golang

Expand All @@ -24,21 +24,29 @@ a CLP IR stream.
"time"

"github.com/klauspost/compress/zstd"
"github.com/y-scope/clp-ffi-go/ffi"
"github.com/y-scope/clp-ffi-go/ir"
)

file, _ := os.Open("log-file.clp.zst")
defer file.Close()
zstdReader, _ := zstd.NewReader(file)
defer zstdReader.Close()
irReader, _ := ir.NewReader(zstdReader)
defer irReader.Close()

irReader, _ := ir.ReadPreamble(zstdReader, 4096)
var err error
for {
// To read every log event replace ReadToContains with
// ReadNextLogEvent(zstdReader)
log, err := irReader.ReadToContains(zstdReader, []byte("ERROR"))
if ir.Eof == err || io.EOF == err {
var log *ffi.LogEventView
// To read every log event replace ReadToContains with Read()
log, err = irReader.ReadToContains("ERROR")
if nil != err {
break
}
fmt.Printf("%v %v", time.UnixMilli(int64(log.Timestamp)), string(log.Msg))
fmt.Printf("%v %v", time.UnixMilli(int64(log.Timestamp)), log.LogMessageView)
}
if ir.EndOfIr != err {
fmt.Printf("Reader.Read failed: %v", err)
}

Building
Expand All @@ -49,15 +57,11 @@ as well as stringify ``Enum`` style types.
1. Install requirements:

a. A C++ compiler that supports C++17
#. CMake 3.5.1 or higher
#. CMake 3.11 or higher
#. The Stringer tool: https://pkg.go.dev/golang.org/x/tools/cmd/stringer

- ``go install golang.org/x/tools/cmd/stringer@latest``

#. ``git submodule update --init --recursive``

- Pull all submodules in preparation for building

#. ``go generate ./...``

- Run all generate directives (note the 3 dots after '/')
Expand All @@ -72,12 +76,12 @@ __ https://github.com/bazelbuild/rules_go/blob/master/docs/go/core/rules.md#go_l

Testing
-------
To run all unit tests run: ``go test ./... -args $(readlink -f clp-ir-stream.clp.zst)``
To run all unit tests run: ``go_test_ir="/path/to/my-ir.clp.zst" go test ./...``

- The ``ir`` package's tests currently requries an existing CLP IR file
compressed with zstd. This file's path is taken as the only argument to the
test and is supplied after ``-args``. It can be an absolute path or a path
relative to the ``ir`` directory.
- Some of the ``ir`` package's tests currently requries an existing CLP IR file
davidlion marked this conversation as resolved.
Show resolved Hide resolved
compressed with zstd. This file's path is taken as an environment variable
named ``go_test_ir``. It can be an absolute path or a path relative to the
``ir`` directory.

Why not build with cgo?
'''''''''''''''''''''''
Expand All @@ -100,5 +104,6 @@ For example, to run the tests using the ``external`` you can run:

.. code:: bash

CGO_LDFLAGS="-L./lib -lclp_ffi_linux_amd64 -lstdc++" \
go test -tags external,test ./... -args $(readlink -f clp-ir-stream.clp.zst)
CGO_LDFLAGS="-L/path/to/external_libs -lclp_ffi_linux_amd64 -Wl,-rpath=/path/to/external_libs" \
go_test_ir="/path/to/my-ir.clp.zst" \
go test -tags external ./...
Loading
Loading